-
-
Notifications
You must be signed in to change notification settings - Fork 678
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
Fix multiple bugs in arrow.get() tzinfo kwarg handling #968
Conversation
Codecov Report
@@ Coverage Diff @@
## master #968 +/- ##
=========================================
Coverage 100.00% 100.00%
=========================================
Files 10 10
Lines 2036 2036
Branches 328 328
=========================================
Hits 2036 2036
Continue to review full report at Codecov.
|
LGTM, but I think this is a more widespread issue @systemcatch. We should be passing along the tz object in all of these other factory methods as well: Lines 234 to 247 in 08705bc
|
Hey @jadchaar I agree with you on the >>> arw=arrow.Arrow(2021, 4, 29, 22, 7, tzinfo="America/Chicago")
>>> arw
<Arrow [2021-04-29T22:07:00-05:00]>
>>> arrow.get(arw, tzinfo="Europe/London")
<Arrow [2021-04-29T22:07:00-05:00]> However the |
@systemcatch I made a few tweaks to comments and made the tzinfo kwarg explicit for the |
Looks good to me Jad, feel free to merge if you're happy with everything. |
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.
LGTM
Pull Request Checklist
Thank you for taking the time to improve Arrow! Before submitting your pull request, please check all appropriate boxes:
tox
ormake test
to find out!).tox -e lint
ormake lint
to find out!).master
branch.If you have any questions about your code changes or any of the points above, please submit your questions along with the pull request and we will try our best to help!
Description of Changes
The
tzinfo
kwarg should now be handled correctly rather than being dropped.fixes #944