Skip to content
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

Doctests in ufuncs should respect __doctest_skip__ #175

Merged
merged 2 commits into from
Feb 25, 2022

Conversation

lpsinger
Copy link
Contributor

No description provided.

Copy link
Contributor

@mhvk mhvk left a comment

Choose a reason for hiding this comment

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

Looks good to me! Definitely makes sense to pass it through the same selection criteria

@pllim
Copy link
Contributor

pllim commented Feb 15, 2022

I think we should document how to use __doctest_skip__ to skip unwanted ufunc doctest?

pllim pushed a commit to pllim/astropy that referenced this pull request Feb 15, 2022
pllim added a commit to pllim/astropy that referenced this pull request Feb 15, 2022
@pllim
Copy link
Contributor

pllim commented Feb 15, 2022

I don't know how I feel about having to add __doctest_skip__ in a test file because we don't even doctest the test functions. Feels like abuse to me. See astropy/astropy#12861

What do others think?

@lpsinger lpsinger requested a review from mhvk February 23, 2022 02:50
@lpsinger
Copy link
Contributor Author

Following recent discussions, I have added back the --doctest-ufunc option, which is disabled by default.

@pllim
Copy link
Contributor

pllim commented Feb 24, 2022

Re-running astropy/astropy#12853 🤞

pllim added a commit to pllim/astropy that referenced this pull request Feb 24, 2022
@pllim
Copy link
Contributor

pllim commented Feb 24, 2022

So.... do we not want to use it over at astropy then? If we do, how do we do it without astropy/astropy#12861 ?

@lpsinger
Copy link
Contributor Author

So.... do we not want to use it over at astropy then? If we do, how do we do it without astropy/astropy#12861 ?

You could revert #12861 if you wanted.

@pllim
Copy link
Contributor

pllim commented Feb 24, 2022

@mhvk , do you really want ufunc doctesting in astropy ?

@mhvk
Copy link
Contributor

mhvk commented Feb 24, 2022

We don't have that many ufuncs, and I don't think any have examples that would be tested, since they are really for internal use. So, certainly no hurry. On the other hand, long-term we might as well test things in case things change.

Copy link
Contributor

@pllim pllim left a comment

Choose a reason for hiding this comment

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

astropy core seems fine with this patch. Thanks!

Shall we merge?

@pllim pllim merged commit feea983 into scientific-python:main Feb 25, 2022
@pllim
Copy link
Contributor

pllim commented Feb 25, 2022

Thanks!

@lpsinger lpsinger deleted the doctest-ufunc-skip branch February 25, 2022 01:26
@lpsinger
Copy link
Contributor Author

Thank you! When do you expect to do the next release of pytest-doctestplus?

@pllim

This comment was marked as resolved.

@pllim
Copy link
Contributor

pllim commented Feb 25, 2022

I'll push a release soon. Stay tuned!

@pllim
Copy link
Contributor

pllim commented Feb 25, 2022

v0.12.0 is released on PyPI.

@bsipocz bsipocz added this to the 0.12.0 milestone Jun 8, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants