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

DateTimeValidator: DateTimeRanges without local_timezone #25

Open
binaryDiv opened this issue Nov 10, 2021 · 0 comments
Open

DateTimeValidator: DateTimeRanges without local_timezone #25

binaryDiv opened this issue Nov 10, 2021 · 0 comments
Labels
enhancement Improvements to existing features or smaller new features question Further information is requested
Milestone

Comments

@binaryDiv
Copy link
Contributor

Currently it is possible to use DateTimeValidator with datetime ranges but without setting local_timezone. This can lead to problems when either the input datetime is a local datetime or the datetime range boundaries are local datetimes (comparing local datetimes with timezoned datetimes will raise a TypeError).

Currently there is this note in the class documentation:

Note: When using datetime ranges, make sure not to mix datetimes that have timezones with local datetimes because those comparisons will raise TypeError exceptions. It's recommended either to use only datetimes with defined timezones (for both input values and the boundaries of the datetime ranges), or to specify the 'local_timezone' parameter (which will also be used to determine the timezone of the range boundary datetimes if they do not specify timezones themselves).

Instead of just warning in the documentation, it might be a good idea to catch these edge cases already at validator creation (i.e. in __init__()): If the DateTimeFormat is one that allows local datetimes AND a range is defined AND local_timezone is NOT set: Raise an InvalidValidatorOptionException.

Not sure though whether there might be usecases where you don't want to define a local_timezone (e.g. you only allow local datetimes (DateTimeFormat.LOCAL_ONLY) and you define a range that has local datetimes as boundaries too).

@binaryDiv binaryDiv added enhancement Improvements to existing features or smaller new features question Further information is requested labels Nov 10, 2021
@binaryDiv binaryDiv added this to the 1.0.0 Release milestone Nov 10, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement Improvements to existing features or smaller new features question Further information is requested
Projects
None yet
Development

No branches or pull requests

1 participant