search

Flowminder / FlowKit

FlowKit: Flowminder CDR analytics toolkit
https://flowminder.github.io/FlowKit/
Mozilla Public License 2.0
86 stars 21 forks source link

Harmonise dealing with date ranges #567

Open maxalbert opened 5 years ago

maxalbert commented 5 years ago

There are many places in the code base where dates are converted to/from strings, date checks are applied, etc. This results in code duplication, but more importantly there is a risk that different parts of the code apply different meaning to the stop/end parameter, or that custom tweaks (such as this) are applied only in certain places but not others.

There should be a single place (e.g. a DateRange class) which contains the logic about how date ranges are interpreted and processed. This class should validate its input and provide support for simple date-based operations. Any other code that needs to deal with dates should accept a DateRange object and use its attributes rather than dealing with date strings directly.

maxalbert commented 5 years ago

In addition, any filtering by hour-of-the-day etc. should be separated out.

jc-harrison commented 4 years ago

One case of inconsistent use of stop parameter is in Multilocation https://github.com/Flowminder/FlowKit/blob/8167c5e321a7718863d6c1930d94efb612d072f2/flowmachine/flowmachine/features/utilities/multilocation.py#L50 where self.stop (which isn't used elsewhere in the code) refers to the latest date in the list of daily locations, not the day after this date (whereas EventTableSubset takes stop to be the day after the end of the interval).