-
-
Notifications
You must be signed in to change notification settings - Fork 1.7k
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
Fix some errors encountered in recent CI builds #9264
Fix some errors encountered in recent CI builds #9264
Conversation
This comment has been minimized.
This comment has been minimized.
I have a few problems with my local <-> remote branches. I planned to make them in one PR but because of the mess-up I rather pushed the changes in separate branches (so I don't have to redo them again). |
Sorry for this mess ... 😓 I think that's it from me. |
With the latest commit all should be fine. And it's about time to think seriously about having |
@bsipocz , not sure how much of these can be cleanly backported. Feel free to re-milestone as you see appropriate. Thanks, all! |
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.
LGTM. Thanks!
These seems to be all fine, nothing much happened to them since 2.0 (thus they were not cleaned up in the first place along with the rest of the changed made about a year ago to accommodate the changes in array representations). |
Merging to clean the CI status asap. |
@@ -28,7 +27,7 @@ | |||
"Flatw0waCDM", "w0waCDM", "wpwaCDM", "w0wzCDM", | |||
"default_cosmology"] + parameters.available | |||
|
|||
__doctest_requires__ = {'*': ['scipy.integrate', 'scipy.special']} | |||
__doctest_requires__ = {'*': ['scipy']} |
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.
FWIW, I think __doctest_requires__ = {'*': ['scipy']}
actually makes more sense, as it is not a namespace package, so if you can import scipy
, chances are you can import the subpackage as well? 🤷♀
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.
the argument for supporting the subpackage version as well is that it can be a new subpackage, e.g. importing astropy doesn't mean that astropy.timeseries
would be available for those tests. Anyway, it's theoretical now as all these scipy functionalities are old enough, and scientific-python/pytest-doctestplus#67 has been opened upstream.
Thank you both for fixing the tests so promptly and cleanly! |
Thanks @bsipocz for finishing this! |
@MSeifert04 - No worries, I should not have put so much trust in the plugin the first place to release it blindly. |
Fix some errors encountered in recent CI builds
Fix some errors encountered in recent CI builds
Fix some errors encountered in recent CI builds
This doesn't work on LTS, and I don't think it's worth the trouble to fix it up given that doctestplus is bundled in that version. |
…l-random-rand Fix some errors encountered in recent CI builds
…l-random-rand Fix some errors encountered in recent CI builds
EDIT: For context, the failure started showing with the latest
pytest-doctestplus
release. Somehow this doctest was never run in previous version ofpytest-doctestplus
.