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
Audit all docstrings for style, typos and outdated info #939
Conversation
Codecov Report
@@ Coverage Diff @@
## master #939 +/- ##
=======================================
Coverage 99.74% 99.74%
=======================================
Files 10 10
Lines 1944 1944
Branches 312 312
=======================================
Hits 1939 1939
Misses 4 4
Partials 1 1
Continue to review full report at Codecov.
|
d12f1f2
to
907a87e
Compare
Is this ready for review @systemcatch? |
Ready for a first review! |
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.
Looks good, but a few tweaks.
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.
Small tweaks, but looks great! This is a good first step. Next step (in a new PR) is to probably integrate a docstring style checker to make sure all of them are consistent, but this is a great start :D!
arrow/arrow.py
Outdated
datetime.datetime(2013, 5, 5, 0, 0, tzinfo=tzutc()) | ||
>>> arrow.Arrow.fromdatetime(dt, dt.tzinfo or 'US/Pacific') | ||
<Arrow [2013-05-05T00:00:00+00:00]> | ||
datetime.datetime(2021, 4, 7, 13, 48) |
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.
Should we make this an tz-aware datetime? Works both ways, just wanted to ask.
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.
Can do!
@@ -1,3 +1,5 @@ | |||
"""Provides internationalization for arrow in over 60 languages and dialects.""" |
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.
Love the addition of the number of locales π!
arrow/util.py
Outdated
""" | ||
Helpful functions used internally within arrow. | ||
|
||
""" |
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.
Single line
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.
Nice.
I had a few comments in a draft review, seems like @jadchaar covered all of them, looks good to me.
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
Closes: #819
Closes: #946