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

Convert TimeZoneField string value to timezone object on assignment #102

Merged
merged 1 commit into from
Dec 9, 2023
Merged

Convert TimeZoneField string value to timezone object on assignment #102

merged 1 commit into from
Dec 9, 2023

Conversation

medmunds
Copy link
Contributor

@medmunds medmunds commented Apr 28, 2023

Add a descriptor_class that deserializes a string TimeZoneField value to a timezone object (pytz timezone or zoneinfo depending on settings) when it is assigned.

As a side effect, invalid (non-blank) timezone names are detected immediately (rather than at save/full_clean time), and will immediately raise a ValidationError.

Closes #57

[There are two commits in this PR. The first, simpler one works for Django 3.0+. The second carries support back to Django 2.2. But if you're dropping Django 2.2 support soon anyway (it exited extended support a year ago), there's no reason to add that extra complication.]

@medmunds
Copy link
Contributor Author

(Incidentally, there seems to be some sort of packaging problem with wrapt which was already breaking the builds.)

@cclauss
Copy link
Contributor

cclauss commented Dec 7, 2023

Please rebase and fix git conflicts.

timezone_field/fields.py Outdated Show resolved Hide resolved
timezone_field/utils.py Outdated Show resolved Hide resolved
Add a descriptor_class that deserializes a string
TimeZoneField value to a timezone object (pytz timezone
or zoneinfo depending on settings) when it is assigned.

As a side effect, invalid (non-blank) timezone names are
detected immediately (rather than at save/full_clean time),
and will immediately raise a ValidationError.

Closes #57
@medmunds
Copy link
Contributor Author

medmunds commented Dec 7, 2023

@cclauss rebased and updated for current code. Looks like the CI workflow approval expired sometime in the past seven months, so someone will need to re-enable it.

@cclauss
Copy link
Contributor

cclauss commented Dec 7, 2023

Below, it says:

1 workflow awaiting approval
This workflow requires approval from a maintainer. Learn more about approving workflows.

@medmunds
Copy link
Contributor Author

medmunds commented Dec 7, 2023

Yes, exactly. (I'm not a maintainer on this repo.)

@cclauss
Copy link
Contributor

cclauss commented Dec 7, 2023

Me neither.

@medmunds
Copy link
Contributor Author

medmunds commented Dec 7, 2023

Oh, sorry @cclauss—just assumed you were a maintainer since you were requesting changes to the PR.

@mfogel is it still the case that "Pull requests happily entertained" to address #57? (And is there some other approach you'd prefer?)

Copy link

codecov bot commented Dec 9, 2023

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (ec2dab7) 98.08% compared to head (028b24c) 98.14%.

Additional details and impacted files
@@            Coverage Diff             @@
##             main     #102      +/-   ##
==========================================
+ Coverage   98.08%   98.14%   +0.06%     
==========================================
  Files           9       10       +1     
  Lines         209      216       +7     
==========================================
+ Hits          205      212       +7     
  Misses          4        4              

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@mfogel
Copy link
Owner

mfogel commented Dec 9, 2023

@medmunds this looks like a great way to address #57 and thanks for the quality contribution.

And @cclauss thanks for the contributions here and also on the other PRs & issues. Much appreciated.

I'll merge this to main now

@mfogel mfogel merged commit 4207dc0 into mfogel:main Dec 9, 2023
64 checks passed
@medmunds
Copy link
Contributor Author

@mfogel thanks for (resurrecting and) maintaining this package!

@medmunds medmunds deleted the feature/convert-string-assignment branch December 11, 2023 17:27
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

Successfully merging this pull request may close these issues.

When saving timezone field with string value it won't get converted to pytz object
3 participants