-
Notifications
You must be signed in to change notification settings - Fork 590
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
st.timezones()
strategy using the stdlib zoneinfo
module
#2682
Conversation
0d52c43
to
0db20ac
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I added some questions, but this looks good!
hypothesis-python/src/hypothesis/strategies/_internal/datetime.py
Outdated
Show resolved
Hide resolved
hypothesis-python/src/hypothesis/strategies/_internal/datetime.py
Outdated
Show resolved
Hide resolved
hypothesis-python/src/hypothesis/strategies/_internal/datetime.py
Outdated
Show resolved
Hide resolved
26cad9f
to
2e5ad5b
Compare
import zoneinfo | ||
except ImportError: | ||
try: | ||
from backports import zoneinfo # type: ignore |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This broke backports.zoneinfo
's test suite (mainly because backports.zoneinfo
's test suite relies on importing backports.zoneinfo
in its own weird way.
Not that you need to change anything, just thought it's kinda funny.
It's not clear how easy it would be to implement such a thing using only the time zone data on disk. You may be able to detect if a given zone is a hard link to another zone, but I'm not actually sure if the hard links are guaranteed to point to the canonical zone (as opposed to a random one). You can probably hard-code a list of existing deprecated zones like anything in |
From one perspective, this is a pretty standard PR adding a new strategy - it's almost identical to the
hypothesis.extra.{dateutil,pytz}.timezones()
strategies, but in thehypothesis.strategies
namespace because the timezones in question come from the standard library.On the other hand, they're only in the latest stdlib, so we also have some logic to manage backports - mostly through a new
hypothesis[zoneinfo]
extra which has only conditional dependencies. This is clearly documented and raises nice errors if you don't have the deps; and since it'll be standard eventually I'd prefer to put it in the right place to start with.Implementation of
allow_aliases
andallow_deprecated
arguments fortimezone_keys()
has been left to a future PR, along with #2414 - this is just the increment to get a minimal useful API out there. Closes #2630.