-
-
Notifications
You must be signed in to change notification settings - Fork 390
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
Test all jplspec code blocks except plots. #2419
Conversation
Codecov Report
@@ Coverage Diff @@
## main #2419 +/- ##
=======================================
Coverage 62.97% 62.97%
=======================================
Files 133 133
Lines 17290 17290
=======================================
Hits 10888 10888
Misses 6402 6402 📣 Codecov can now indicate which changes are the most critical in Pull Requests. 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.
Thank you! one minor comment, otherwise this is ready to go in.
docs/jplspec/jplspec.rst
Outdated
... min_strength=-500, | ||
... molecule="H2O", | ||
... parse_name_locally=True) | ||
>>> print([1, 2, 3]) # doctest: +ELLIPSIS |
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.
is this for debug? Also, ELLIPSIS should be opted-in by default. If not that's an infrabug that we should fix on the config level.
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.
(Oh, now I see that my comment in the issue was ambiguous, I only meant to use ...
in the outputs for ELLIPSIS, but if you do that it should work out of the box.
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.
Oops! I was having trouble with this test, and I thought it was about getting ... to work, so I thew in a debug test to figure it out. It turns out I was printing the result table, but the test string was from __repr__
. Then I forgot to remove the debug test. I'll fix that and verify that the directive isn't needed here.
Thank you @mkelley! |
Address #2408 .