Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Zmanim vary by time when using a datetime object as input #18

Open
hudcap opened this issue Aug 7, 2023 · 2 comments
Open

Zmanim vary by time when using a datetime object as input #18

hudcap opened this issue Aug 7, 2023 · 2 comments

Comments

@hudcap
Copy link

hudcap commented Aug 7, 2023

Different times yield different zmanim.
MWE:

import datetime as dt
from zmanim.util.geo_location import GeoLocation
from zmanim.zmanim_calendar import ZmanimCalendar
location = GeoLocation('Lakewood, NJ', 40.0721087, -74.2400243, 'America/New_York', elevation=15)
print(ZmanimCalendar(geo_location=location, date=dt.datetime(2023,8,7)).shkia())
print(ZmanimCalendar(geo_location=location, date=dt.datetime(2023,8,7,1)).shkia())

Even worse, when no date is passed, the default is a datetime object with the current time, so

print(ZmanimCalendar(geo_location=location).shkia())

will yield different, incorrect times throughout the day.

As per @pinnymz, ZmanimCalendar should require a date object.
While a datetime object can be coerced to a date, since the caller may possibly want the next day's zmanim (e.g., if it's after nightfall), we agreed that the input should be restricted to date object, placing the onus on the caller to decide which date.

PR to follow.

@hudcap
Copy link
Author

hudcap commented Aug 7, 2023

This will break backwards compatibility, but every datetime object has a date() method that will return a date, so it shouldn't be too difficult to update old code.

@pinnymz Should we allow datetime objects set to midnight, in an attempt to preserve some backwards compatibility?

@hudcap
Copy link
Author

hudcap commented Aug 7, 2023

The date object is imported from the datetime library, but date is also used as a parameter, which can create problems.

@pinnymz Any objections if I switch to import datetime as dt and then use dt.date, dt.datetime, etc.?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant