arrow-py / arrow

🏹 Better dates & times for Python
https://arrow.readthedocs.io
Apache License 2.0
8.64k stars 667 forks source link

feat(1090/default_tz): Added possibility to override default UTC - #1… #1122

Open morcef opened 1 year ago

morcef commented 1 year ago

Pull Request Checklist

Thank you for taking the time to improve Arrow! Before submitting your pull request, please check all appropriate boxes:

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

https://github.com/arrow-py/arrow/issues/1090 - Added possibility to override default UTC timezone. I don't believe this should be breaking anything, but I appretiate any feedback as this is my first PR on such a project.

Closes: #1090

codecov[bot] commented 1 year ago

Codecov Report

Merging #1122 (b087369) into master (f8f3068) will not change coverage. The diff coverage is 100.00%.

@@            Coverage Diff            @@
##            master     #1122   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files           10        10           
  Lines         2325      2336   +11     
  Branches       449       446    -3     
=========================================
+ Hits          2325      2336   +11     
Impacted Files Coverage Δ
arrow/api.py 100.00% <ø> (ø)
arrow/arrow.py 100.00% <100.00%> (ø)
arrow/constants.py 100.00% <100.00%> (ø)
arrow/factory.py 100.00% <100.00%> (ø)
arrow/util.py 100.00% <100.00%> (ø)

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

morcef commented 1 year ago

Thank you @anishnya for approving the workflow. I must admit that I am new to the whole contribution part, so this is my first contribution in form of an actual PR. If I understood the previous workflow, it failed on two linting errors in two files, and the coverage was reduced. I have tried to fix theses in my new commits. Please let me know if there is anything else that needs improvement/fixing :)

anishnya commented 1 year ago

the previous workflow, it failed on t

No worries. Thank you for contributing! Please feel free to ask us any questions and we'll be more than happy to help! :)

morcef commented 1 year ago

@anishnya , my apologies, I did not catch the isort and black failures. I believe I should have sorted out all the errors here now 🤞

morcef commented 1 year ago

Oh my! I did not notice that docstring was removed.. @anishnya , I am not fully sure what caused the segmentation fault in the macos tests, but hopefully this was just a glitch in the system?

morcef commented 1 year ago

Hi @anishnya, There seems to have appeared a conflict with the documentation here. I guess the documentation has been updated since I pushed my code. Is there much going on with the code base these days? Has there been/is there any chance to take a look at this PR? Thanks

krisfremen commented 1 year ago

Hi @anishnya, There seems to have appeared a conflict with the documentation here. I guess the documentation has been updated since I pushed my code. Is there much going on with the code base these days? Has there been/is there any chance to take a look at this PR? Thanks

Hey @morcef, we did move things around in the documentation recently to improve readability.

I try to get stuff reviewed and merged in when free time pops up. I will take a look at this PR in the next few days.

morcef commented 1 year ago

Thank you @krisfremen :) In regards to the documentation conflict, would you like me to look into that, or would it be better for you to just place my changes according to your recent improvement?