Skip to content

Conversation

@ayshih
Copy link
Contributor

@ayshih ayshih commented Feb 17, 2021

This PR fixes a bug where the handling of IGNORE_WARNINGS/SHOW_WARNINGS flags in module docstrings is entirely skipped over if --remote-data=any is specified (for concurrent use of the pytest-remotedata plugin).

@pllim
Copy link
Contributor

pllim commented Feb 17, 2021

Huh, why does this sound familiar? Did #148 not work for you?

@pllim pllim requested a review from saimn February 17, 2021 18:02
@pllim pllim added the bug label Feb 17, 2021
@pllim pllim added this to the 0.10.0 milestone Feb 17, 2021
@ayshih
Copy link
Contributor Author

ayshih commented Feb 17, 2021

#148 fixes a loosely related bug for RST files, but this bug is for module docstrings.

@saimn
Copy link
Contributor

saimn commented Feb 17, 2021

Oh indeed, I saw this if once and did not understand why it was like this. Good catch, and thanks for the PR @ayshih !

@saimn saimn merged commit e000c6f into scientific-python:master Feb 17, 2021
@saimn
Copy link
Contributor

saimn commented Feb 17, 2021

While pushing the merge button I was thinking that it would be great if there was a test for this ;-). A bit late for this PR but if you have a bit more time to spend on this, would be appreciated.

@ayshih
Copy link
Contributor Author

ayshih commented Feb 17, 2021

I contemplated adding a test, but it's a little weird because I don't believe you can actually feed in --remote-data=any as a command-line option without also having pytest-remotedata installed (i.e., it errors). Do you want that dependency in the test suite? Or should the test instead inject the command-line option after pytest has already checked for validity of the arguments?

@pllim
Copy link
Contributor

pllim commented Feb 17, 2021

I think a test can be added as part of #137 or as a follow up to it?

@saimn
Copy link
Contributor

saimn commented Feb 17, 2021

Ah right, it's not there yet. Yes, could be in #137 then.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants