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

ENH: adding doctest-requires-all directive #280

Merged
merged 4 commits into from
Jan 24, 2025

Conversation

bsipocz
Copy link
Member

@bsipocz bsipocz commented Jan 23, 2025

The need for this came up again recently in astroquery, so finally I got down to do it.

To close #183

@bsipocz bsipocz added this to the v1.4.0 milestone Jan 23, 2025
@bsipocz bsipocz requested a review from pllim January 23, 2025 19:20
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.

Thanks! I wonder if this will also be useful for #279.

Just some minor comments.

pytest_doctestplus/plugin.py Outdated Show resolved Hide resolved
README.rst Show resolved Hide resolved
README.rst Outdated
@@ -336,6 +336,11 @@ the package's ``setup.cfg`` file. The syntax for this option is a list of

Multiple requirements can be specified if separated by semicolons.


Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change

Co-authored-by: P. L. Lim <2090236+pllim@users.noreply.github.com>
@bsipocz bsipocz force-pushed the doctest-requires-all branch from 0044a42 to 74cebb8 Compare January 23, 2025 21:56
@bsipocz
Copy link
Member Author

bsipocz commented Jan 23, 2025

Not sure about #279, if anyone wants to dive into it be my guest. But this is very similar to #146, so I'm opening a PR for that, too.

Comment on lines +453 to +455
This is a narrative line, before another doctest snippet

>>> import foobar
Copy link
Contributor

Choose a reason for hiding this comment

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

Fix formatting to reflect typical real use case.

Suggested change
This is a narrative line, before another doctest snippet
>>> import foobar
This is a narrative line, before another doctest snippet::
>>> import foobar

Copy link
Member Author

Choose a reason for hiding this comment

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

This should not matter, all the snippets should be collected no matter if there are :: or correct number of spaces before the >>>.

@bsipocz
Copy link
Member Author

bsipocz commented Jan 24, 2025

I go ahead and merge this now, so the other PR doesn't have to be rebased.

@bsipocz bsipocz merged commit d620c96 into scientific-python:main Jan 24, 2025
19 checks passed
@bsipocz bsipocz deleted the doctest-requires-all branch January 24, 2025 01:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

ENH: doctest-requires-all to declare dependencies for all doctests
2 participants