Skip to content

DOC: Show constructor arguments for some classes in pd.series.offsets #61605

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

Open
wants to merge 16 commits into
base: main
Choose a base branch
from

Conversation

Dr-Irv
Copy link
Contributor

@Dr-Irv Dr-Irv commented Jun 8, 2025

This contributes to #52431

In that issue, the goal is to show the arguments for the constructors for the various offsets, which are all in pyx files.

The problem is that sphinx doesn't pick up the arguments for __init__() in those files. It seems that if you have class that is a not a cdef class, you have to have a __new__() method so the docs show up. This PR does this for YearEnd, by creating a cdef _YearEnd class. and then YearEnd is a regular class with __new__() calling __new__() of _YearEnd. So, if you preview the docs from this MR, you will see that YearEnd now shows its constructor.

If this PR is accepted, then we can get the community to do the work of separating the classes into "public" and "private" ones and adding adding the __new__() methods to the other documented offset classes and they can also add a Parameters section to those classes. Note that for YearEnd, I had to change the docs from Attributes to Parameters to make it so that the docstrings are validated.

f"\'{c_PERIOD_AND_OFFSET_DEPR_FREQSTR.get(name)}\'"
f" instead.",
f"\'{c_PERIOD_AND_OFFSET_DEPR_FREQSTR.get(name)}\' "
f"instead.",
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@Dr-Irv this and the following change have for some time caused pre-commit to fail for me locally.

great that this is being fixed here. Thanks.

Any idea why CI hasn't picked this up before now?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No, I was surprised as well.

@Dr-Irv
Copy link
Contributor Author

Dr-Irv commented Jun 8, 2025

/preview

@datapythonista
Copy link
Member

No strong opinions about this, but if the examples need the okexcept on windows sounds like this change is breaking things on windows, no? I guess I'm missing something, if you can clarify. But it would be better to fix the underlying problem, I don't think we should add okexcept unless we document exceptions.

@Dr-Irv
Copy link
Contributor Author

Dr-Irv commented Jun 9, 2025

No strong opinions about this, but if the examples need the okexcept on windows sounds like this change is breaking things on windows, no? I guess I'm missing something, if you can clarify. But it would be better to fix the underlying problem, I don't think we should add okexcept unless we document exceptions.

No, the change is needed to just build the regular documentation (without this change) on Windows. It has something to do with how sphinx calls cython and that doesn't seem to work right on Windows. Not sure exactly what the problem is.

@Dr-Irv
Copy link
Contributor Author

Dr-Irv commented Jun 22, 2025

No strong opinions about this, but if the examples need the okexcept on windows sounds like this change is breaking things on windows, no? I guess I'm missing something, if you can clarify. But it would be better to fix the underlying problem, I don't think we should add okexcept unless we document exceptions.

I found a better fix in #61686 . So the okexcept is not there any more.

@Dr-Irv
Copy link
Contributor Author

Dr-Irv commented Jun 22, 2025

/preview

1 similar comment
@Dr-Irv
Copy link
Contributor Author

Dr-Irv commented Jun 23, 2025

/preview

Copy link
Contributor

Website preview of this PR available at: https://pandas.pydata.org/preview/pandas-dev/pandas/61605/

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants