Refactor datetime code

[matthijskooijman]

Currently, the codebase has a distinction between:

• Regular dates/datetimes, which refer to the actual moment in time of a fact start or end.
• hday or "hamster days", which refer to the day that a fact (start) belongs to. This is mostly the same as the regular day, but for past-midnight times, the hday is one day less than the regular date (i.e. past-midnight times are associated with the preceding day). These hamster days are represented by a custom hday subclass of date.

This distinction is sometimes non-obvious, and mixing these leads to problems with past-midnight facts. It would be good to change some variable names to say day or hday to make this more explicit. In particular:

1. CustomFactController.date refers to the hamster day selected in the edit activity window (i.e. the date shown in the dayline at the top).
2. Fact.date refers to the hamster day for a given fact.
3. Probably more places.

I'm still a bit on the fence on whether to prefer day or hday for these variables. In https://github.com/ederag/hamster/commits/fix-calendar-offset @ederag used day to fix 1. above, and day is also used as part of default_day in various places. On the other hand, using hday looks a bit more specific, which could make it more obvious to casual readers of the code that this is not just a date variable (and grepping the code for hday would more readily turn up the hday class). Any thoughts on this?

A second distinction that is made is between:

• Python's date, time and datetime objects (referred to as pdt.date, pdt.time and pdt.datetime in some places)
• Hamster's own subclasses of those, which add some methods.

I guess the idea is to use the Hamster-specific classes as much as possible, only converting to and from the pdt types when talking to external code (i.e. through DBus), so I don't think any variable renames are needed here.

I do wonder if the upsides of these extra types is worth the extra complexity (and the fragility from mixing up types). It seems that a large part of the code for these types is to ensure that any operations on the custom code will still return the custom types and handle conversion to and from the standard types (in other words, all that code would not be needed if these types did not exist). Looking at what these types actually add:

1. Methods to parse strings and convert other values to these new types. These all seem to be class methods, so these could just as easily be implemented as functions without needing to change the actual types of all date/time values.
2. The time and datetime types are modified to truncate to whole minutes (by forcing seconds and microseconds to 0 in the consructor). This could possibly be done explicitly as well (in every place that currently converts from e.g. pdt.time to time, it the deprecated hamster_round function suggests this used to be done before), but I guess there is some value in doing this automaticaly.
3. A method to convert datetime to hday. This could just be a matter of adding an appropriate constructor to the hday class, I guess, or using a global function (this seems to have been the old approach, looking at the deprecated datetime_to_hamsterday() function)
4. Explicitness about whether a particular value is a raw external value, or has been converted to a "hamster" equivalent. This allows internal methods to either assert that they get a proper converted value, or alternatively handle both hamster and pdt versions and automatically convert when they get a pdt value. However, looking at the the above, "converting" here seems to just mean rounding, so any checking could be a matter of checking for 0 seconds/microseconds and automatically converting could be a matter of unconditionally rounding (which is idempotent, so no harm in doing it when not strictly needed).

Note that I think the hday type is certainly useful, since that actually changes the meaning of the value, so it is good to track that. but for the other types, I'm not so sure.

[ederag]

You have too little understanding of the infrastructure to tackle any "refactoring" yet.
I can't believe you dare look this way when you still struggle with the user interface (starting with #590).

What you call "fragility" comes only from your current (you'll see, eventually) shallowness.
For example, you said (emphasis is mine):

I have the feeling that this entire code might need a more thorough redesign to become a bit less fragile.

or in #627 (comment):

things seem considerably more broken than I thought.

I often let it pass, to reduce conflict, but it has the deterrent property to mislead others
(for instance no one else tested #597 partly because of the bad comments from the new team)

---

The other types were part of a smooth transition
towards a timezone-aware hamster (#346, #434).
Clients would just replace from python import datetime with from hamster import datetime,
without any other change to their code.
Then when hamster would be time-zone aware (in a few years),
they would benefit from the improvement without any change at all,
and work with all versions of hamster, starting with v3.

[matthijskooijman]

You have too little understanding of the infrastructure to tackle any "refactoring" yet.

Agreed that I personally have not a full understanding of the codebase yet, which is also why I'm not diving into such a refactoring immediately, but I did want to put down my thoughts for them to not get lost and for others to chime in.

Anyway, thanks for your thoughts on the datetime issue. Having a drop-in replacement for client code indeed seems like an advantage, but I'm not sure yet how this fits into the bigger picture.