Between filter wants me to work at new year

[huehnerhose]

Problem

$filtered = $yasumi->between(
    new DateTime(($year).'-01-01'),
    new DateTime(($year + 1).'-01-01')
);

Returns the 2017-01-01 as holiday only if the DateTime Timezone is the same as the local timezone, not if I use an DateTime with UTC.

Background

Internally I use a wrapped DateTime class fixed to UTC with Hours:minutes to 0 to identify a single day. My local timezone is "Europe/Berlin". So DateTime defaults to 2017-01-01 00:00:00+1 and my class defaults to 2017-01-01 00:00:00Z.

So these two describe the following "moments in spacetime written without any offset" DateTime (Europe/Berlin): 2016-12-31 23:00:00Z MyClass: 2017-01-01Z

Yasumi creates the holidays based on the timezone given in the provider class. So germanys new year is: 2017-01-01 00:00:00+1 or 2016-12-31 23:00:00Z. The between filter function now checks if

$holiday >= $this->start_date && $holiday <= $this->end_date

which must be denied, when $this->start_date = MyUTCDay('2017-01-01') because

2016-12-31 23:00:00Z >= 2017-01-01 00:00:00Z

is false

Discussion

I can understand the way you think, but I would like to advocate for the following idea:

If the holiday intersects with the given between barrier include it into the result set.

On the pro side I can be sure to get all holidays in my time frame. So I could ask the result set for any moment inside its timeframe if this moment is a holiday, regardless of day.

On the against side I could end in getting too many holidays which results in unesecarry unwanted holidays in the result set.

Conclusion

I'd like to advocate for an option to get an inclusive between filter or an option to query for a day without a timezone which then defaults to the provider class timezone.

[stelgenhof]

Thank you very much for submitting this issue. As I understand the issue, I am not sure I follow why you want to use a UTC date time object using the BetweenFilter.

Could you perhaps provide some use case so I understand it better?

[jvalecillos]

I bumped with this error as well and I can confirm. This is how I tried to reproduce:

$timeZone = new \DateTimeZone("Europe/Berlin");
$yasumiDate = new Yasumi\Holiday(
    "New Year's Day",
    [],
    new DateTime("2018-01-01 00:00:00.000000", $timeZone),
    Yasumi\Holiday::DEFAULT_LOCALE,
    Yasumi\Holiday::TYPE_OFFICIAL
);

$regularDate = new \DateTime("2017-12-19 00:00:00.000000", $timeZone);

var_dump($yasumiDate >= $regularDate); // It returns true but false when using the BetweenFilter

I think that the error is more general than just the new year's day. In my snippet both time zones are equal. I think is more in how PHP is interpreting the comparison against the custom class Yasumi\Holiday.

[stelgenhof]

Thanks @jvalecillos for the example. Let me see if I can replicate and test it out :) Could you then also show me an example where the BetweenFilter fails?

[stelgenhof]

Still not sure what the issue is you are experiencing. I am trying to reproduce it but without success. If I run the following snippet:

$year = 2017;
$yasumi = Yasumi\Yasumi::create('Netherlands', $year);

$filtered = $yasumi->between(
    new \DateTime($year . '-01-01'),
    new \DateTime($year . '-12-31')
);

$betweenHolidays = iterator_to_array($filtered);

foreach ($filtered as $holiday) {
    echo $holiday->getName().PHP_EOL;
}

I get all the holidays defined for the date range I have provided, even without providing any timezone.

My output is:

Epiphany
Valentine's Day
Carnival
Carnival
Carnival
Ash Wednesday
Summertime
Good Friday
Easter Sunday
Easter Monday
Kings Day
International Workers' Day
Commemoration Day
Liberation Day
Mother's Day
Ascension Day
Whitsunday
Whitmonday
Father's Day
Prince's Day
World Animal Day
Wintertime
Halloween
St. Martin's Day
St. Nicholas' Day
Christmas
Second Christmas Day

[c960657]

The code in the latest comment depends on the default timezone. If PHP is configured to use a different timezone offset than Germany, it will miss holidays on either 1 January or 31 December.

The following example is more explicit. The first between() returns 1 result, the second 0 results.

$holidays = Yasumi\Yasumi::create('Germany', 2017);
$holidays->between(
    new DateTime('2017-01-01 Europe/Berlin'),
    new DateTime('2017-01-01 Europe/Berlin'));
$holidays->between(
    new DateTime('2017-01-01 Europe/London'),
    new DateTime('2017-01-01 Europe/London'));

When using DateTime instance to represent a whole day rather than a point in time, I think it makes sense to treat two instances as equal, if $date1->format('Y-m-d') == $date2->format('Y-m-d') instead of the stricter $date1 == $date2 (i.e. that they represent the same Unix timestamp down to the second). AbstractProvider::isHoliday() already does that. I suggest changing BetweenFilter to use that logic also.

[stelgenhof]

What could be a good use case for this? I am trying to understand in what situation one would want to retrieve holidays from one provider but with different timezone.

[c960657]

I don't think there is a specific use case, just an easier API that is more consistent with AbstractProvider::isHoliday().

Right now you need to know the timezone used by a given provider. There is no public method for getting this, but even if there were, new DateTime('2018-01-01') is easier to type than new DateTime('2018-01-01', new DateTimeZone($provider->getTimeZone())).

As long as you are working with providers in your own timezone, the current behaviour is not a big issue. But if you are getting holidays for a different region than the one your server is in, or if you are getting holidays for multiple regions at a time, the current API is a bit cumbersome.

In general, if a DateTime instance is used to represent a date rather than a point in time, I would assume only the date part is considered, so that new DateTime('2018-01-01 00:00:00') andnew DateTime('2018-01-01 00:00:00') represent the same date, and new DateTime('2018-01-01 America/New_York') and new DateTime('2018-01-01 America/Los_Angeles') represent the same date.

For instance, the dates returned by the USA provider returns DateTime instances with timezone America/New_York, but that doesn't mean that the holidays don't start at midnight in Los Angeles.

[stelgenhof]

Thanks for the additional information!

In Yasumi, any DateTime instance should represent a date rather than point in time. To my understanding there is no such situation that there could be holidays recognized by timezone (i.e. hour/minutes) within a country.

I think we can then indeed adjust the logic in the BetweenFilter in the same manner as used in the isHoliday function.