Skip to content

TST: Ignore test warnings in modeling#9538

Merged
bsipocz merged 5 commits intoastropy:masterfrom
pllim:modeling-tst-warn
Nov 7, 2019
Merged

TST: Ignore test warnings in modeling#9538
bsipocz merged 5 commits intoastropy:masterfrom
pllim:modeling-tst-warn

Conversation

@pllim
Copy link
Member

@pllim pllim commented Nov 5, 2019

Description

This pull request is to address modeling tests emitting AstropyDeprecationWarning when testing deprecated BlackBody1D and the MexicanHat* models, and other warnings.

Part of #7928

@pllim pllim added this to the v4.1 milestone Nov 5, 2019
@pllim pllim requested a review from nden November 5, 2019 21:43
@pllim pllim changed the title TST: Ignore AstropyDeprecationWarning in _allmodels TST: Ignore test warnings in modeling Nov 5, 2019
@pllim pllim force-pushed the modeling-tst-warn branch from 744ddd7 to b35a2ec Compare November 6, 2019 17:59
@pllim
Copy link
Member Author

pllim commented Nov 6, 2019

Ready for review!

Copy link
Contributor

@nden nden left a comment

Choose a reason for hiding this comment

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

Thanks for all this work! I feel like some of the warnings should be caught in the code and never reach the user but it's a task for a different PR.


@pytest.mark.filterwarnings("ignore:BlackBody provides the same capabilities")
@pytest.mark.filterwarnings(
r"ignore:Input contains invalid wavelength/frequency value\(s\)")
Copy link
Contributor

Choose a reason for hiding this comment

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

I'm not sure if this warning should be ignored or caught. Although only one of the values is raising it so may be it should have been a separate test.

This comment was marked as resolved.

Copy link
Member Author

Choose a reason for hiding this comment

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

@nden , I catch them like you suggested.

self.y1 = np.arange(1, 10, .1)
self.y2, self.x2 = np.mgrid[:10, :8]

@pytest.mark.filterwarnings(r'ignore:.*:RuntimeWarning')
Copy link
Contributor

Choose a reason for hiding this comment

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

For some reason I can't find a test log that shows the warnings. Can you point me to one. I'm curious what raises this RuntimeWarning.

Copy link
Member Author

Choose a reason for hiding this comment

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

You can see it in the log of #7928. For example, starting at https://travis-ci.org/astropy/astropy/jobs/608405975#L2929

@nden
Copy link
Contributor

nden commented Nov 6, 2019

Looks like the failures are not related. Feel free to merge if you agree.

@pllim pllim force-pushed the modeling-tst-warn branch from b35a2ec to 246d32c Compare November 6, 2019 22:00
@pllim pllim added the zzz 💤 merge-when-ci-passes Do not use: We have auto-merge option now. label Nov 6, 2019
@pllim
Copy link
Member Author

pllim commented Nov 6, 2019

Thanks for the review, @nden ! The mpldev failure is unrelated and reported at #9551.

@pllim
Copy link
Member Author

pllim commented Nov 6, 2019

p.s. @bsipocz , if you think this can be in 4.0, feel free to re-milestone. I have no strong opinion either way.

@nden
Copy link
Contributor

nden commented Nov 7, 2019

Looks good. Please squash all commits.

@bsipocz bsipocz modified the milestones: v4.1, v4.0 Nov 7, 2019
@bsipocz
Copy link
Member

bsipocz commented Nov 7, 2019

Thanks @pllim!

@bsipocz bsipocz merged commit d0df01f into astropy:master Nov 7, 2019
@pllim
Copy link
Member Author

pllim commented Nov 7, 2019

Re: squashing -- ops, saw the comment too late. Thanks for the review, @nden!

@pllim pllim deleted the modeling-tst-warn branch November 7, 2019 15:38
@bsipocz
Copy link
Member

bsipocz commented Nov 7, 2019

5 commits for ~200 lines of change is super reasonable, so I didn't feel like enforcing the squash but rather get on with the merge.

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