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

Speed up registry doctest #567

Merged
merged 2 commits into from
Jul 2, 2024
Merged

Conversation

msdemlei
Copy link
Contributor

This is using other constraints and services in order to transfer less data and perhaps have faster queries.

msdemlei added 2 commits June 28, 2024 14:24
This is now about 20 seconds where I sit...

...provided we don't happen to hit broken obscore services.  My take on that
would be to throw not run the global obscore things any more as soon as the
global dataset discovery stuff is merged -- this does all that a lot better,
with proper timeout and possibly other services as well.
Copy link
Member

@bsipocz bsipocz left a comment

Choose a reason for hiding this comment

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

Minor comment, otherwise looks good.

Thanks!

Comment on lines +38 to +39

.. doctest::
Copy link
Member

Choose a reason for hiding this comment

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

no need for this directive, code with >>> are picked up for doctesting

Suggested change
.. doctest::

@bsipocz
Copy link
Member

bsipocz commented Jun 28, 2024

while checking this locally, I discovered some weird behaviour/bug of the tests, I'll open a separate issue for that as the behaviour is present on main, too.

@msdemlei
Copy link
Contributor Author

msdemlei commented Jul 1, 2024 via email

@bsipocz
Copy link
Member

bsipocz commented Jul 1, 2024

We don't use sphinx for picking up doctests and its plumbing is somewhat different than the pytest ecosystem and plugins we use (e.g. some sphinx doctest directives are known to not work with pytest/pytest-doctestplus and similarly, some doctestplus things that we rely on are known not working with sphinx.). So, while most things may work without issues, I wouldn't be surprised if you see doctest failures in CI even if they pass with the sphinx system locally.

So, having this one .. doctest:: is simply superfluous but doesn't break anything, so can stay, but you will need to keep using >>> for the code snippets otherwise they won't be picked up by the system we use, even though sphinx doctest may be happy with just using the doctest directive.

@bsipocz bsipocz merged commit f2d6b7d into astropy:main Jul 2, 2024
10 of 11 checks passed
@bsipocz bsipocz modified the milestones: v1.6, v1.5.3 Oct 14, 2024
bsipocz added a commit that referenced this pull request Oct 14, 2024
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.

3 participants