-
-
Notifications
You must be signed in to change notification settings - Fork 19.4k
details on weekday argument for DateOffset docs #54332
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
Conversation
|
Hey @Detoro . Can you try commenting |
|
pre-commit.ci autofix |
|
/preview |
|
No preview found for PR #54332. Did the docs build complete? |
MarcoGorelli
left a comment
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.
thanks for your PR
this addresses part of the issue - the other part is about the link
in See also, could you replace dateutil.relativedelta.relativedelta with https://dateutil.readthedocs.io/en/stable/relativedelta.html please?
thanks
pandas/_libs/tslibs/offsets.pyx
Outdated
| weekday : int {0, 1, ..., 6}, default 0 | ||
| A specific integer for the day of the week. | ||
| - 0 is Monday | ||
| - 1 is Tuesday | ||
| - 2 is Wednesday | ||
| - 3 is Thursday | ||
| - 4 is Friday | ||
| - 5 is Saturday | ||
| - 6 is Sunday. |
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.
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.
Working with @Detoro on this - we'd like to view the built HTML but have been encountering some issues.
Namely, after running python make.py --single pandas.tseries.offsets.DateOffset in the doc directory, we don't see any output in directory reference/api for DateOffset. We're getting this warning from Sphinx:
/mnt/c/Users/batiu/OneDrive/Desktop/pandasContribution/pandas-toro/doc/source/index.rst:31: WARNING: autosummary references excluded document 'pandas.tseries.offsets.DateOffset'. Ignored.
Are we building this class correctly, and if so why is the class's documentation being excluded by "autosummary references"?
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.
@MarcoGorelli PR is updated, can you take another look? We'll squash the commits as well.
We were planning to do this in a follow up PR. Do you prefer if both parts are done in separate PRs, or a single PR? |
|
/preview |
|
No preview found for PR #54332. Did the docs build complete? |
|
pre-commit.ci autofix |
|
@mroeschke could you take a look? |
MarcoGorelli
left a comment
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.
thanks, this is close
if this doesn't fully close the issue, can you replace closes with towards in the pull request description please?
| previous midnight. | ||
| **kwds | ||
| Temporal parameter that add to or replace the offset value. | ||
| weekday : int {0, 1, ..., 6}, default 0 |
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 this go before kwds? that should typically be the last argument
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.
Done.
MarcoGorelli
left a comment
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.
thanks @Detoro , lgtm on green
|
why did you close this? |
|
my bad @MarcoGorelli |


weekdayargument and has incorrect link forrelativedelta#52650