Skip to content

Fix incorrect test discovery due to np.arctan2 being left as a global#12858

Merged
pllim merged 1 commit intoastropy:mainfrom
lpsinger:del-ufunc
Feb 15, 2022
Merged

Fix incorrect test discovery due to np.arctan2 being left as a global#12858
pllim merged 1 commit intoastropy:mainfrom
lpsinger:del-ufunc

Conversation

@lpsinger
Copy link
Contributor

This should fix the CI failure in #12853.

@mhvk
Copy link
Contributor

mhvk commented Feb 15, 2022

Nice sleuthing! I was at first surprised this file gets checked at all, since it is not documented, but only then realized that pytest collection and sphinx documentation are of course completely unrelated. It still seems odd, however, that a doctest would run on something that is not actually defined in the module. But checking, I see that there is just no way to see where the ufunc is actually defined.

So, am happy to go with this - though do please add a comment!

Also, I guess if we will eventually use the new pytest-doctestplus functionality in LTS, this should be backported?

p.s. Longer term, it might more sense to move all those additions to a function, as is done for scipy.py, but that can be in another PR...

@pllim

This comment was marked as outdated.

@pllim
Copy link
Member

pllim commented Feb 15, 2022

Oh wait... My PR is not supposed to be merge. Never mind... 🤪

Copy link
Member

@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!

@pllim pllim merged commit cd730f9 into astropy:main Feb 15, 2022
meeseeksmachine pushed a commit to meeseeksmachine/astropy that referenced this pull request Feb 15, 2022
@lpsinger lpsinger deleted the del-ufunc branch February 15, 2022 18:40
pllim added a commit that referenced this pull request Feb 15, 2022
…858-on-v5.0.x

Backport PR #12858 on branch v5.0.x (Fix incorrect test discovery due to np.arctan2 being left as a global)
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