Skip to content
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

Remove warning filters that are no longer needed for testing #2536

Merged
merged 3 commits into from
Oct 27, 2023

Conversation

rosteen
Copy link
Collaborator

@rosteen rosteen commented Oct 25, 2023

Does what it says on the tin.

@rosteen rosteen added this to the 3.8 milestone Oct 25, 2023
@rosteen rosteen added the no-changelog-entry-needed changelog bot directive label Oct 25, 2023
@codecov
Copy link

codecov bot commented Oct 25, 2023

Codecov Report

All modified and coverable lines are covered by tests ✅

see 3 files with indirect coverage changes

📢 Thoughts on this report? Let us know!.

@rosteen rosteen added testing trivial Only needs one approval instead of two labels Oct 25, 2023
@rosteen
Copy link
Collaborator Author

rosteen commented Oct 25, 2023

Marking this trivial since the only requirement is that the tests pass.

Not sure how this affected coverage, I think Codecov is hallucinating.

@@ -127,26 +127,12 @@ text_file_format = "rst"
addopts = "--doctest-rst --import-mode=append"
filterwarnings = [
"error",
"ignore:numpy\\.ufunc size changed:RuntimeWarning",
Copy link
Contributor

Choose a reason for hiding this comment

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

Need to keep the ufunc one. It comes up once in a while depending on how CI installed stuff and in what order.

@pllim
Copy link
Contributor

pllim commented Oct 25, 2023

I think you removed too many...

E   DeprecationWarning: datetime.datetime.utcfromtimestamp() is deprecated
and scheduled for removal in a future version.
Use timezone-aware objects to represent datetimes in UTC:
datetime.datetime.fromtimestamp(timestamp, datetime.UTC).

@rosteen
Copy link
Collaborator Author

rosteen commented Oct 25, 2023

I think you removed too many...

Blast. I'll come back to this tomorrow, I was testing locally but didn't have all the dev dependencies.

@rosteen rosteen marked this pull request as draft October 25, 2023 21:26
@pllim
Copy link
Contributor

pllim commented Oct 25, 2023

datetime.datetime.utcfromtimestamp() is actually from Python 3.12 but yeah. 😬

Thanks for tackling this!

Temporarily remove a few tests

Put these back
@rosteen rosteen force-pushed the test-unignore-warnings branch from 5003c24 to fbbf77b Compare October 26, 2023 15:04
@rosteen
Copy link
Collaborator Author

rosteen commented Oct 26, 2023

The 3.12 dev test now hits a real error from pandas - my attempt at getting a local env set up with all the dev requirements was a mess, so I can't 100% confirm this is good to go. I'll see if pandas-dev/pandas#55707 gets merged quickly to resolve the pandas failure and put this on the back-burner until next week.

@pllim
Copy link
Contributor

pllim commented Oct 26, 2023

Yeah probably save us time to just wait out the pandamonium.

@mroeschke
Copy link

Just merged in pandas-dev/pandas#55707 so hopefully this should show up in tomorrow's wheels

@pllim pllim marked this pull request as ready for review October 27, 2023 14:42
Copy link
Contributor

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

devdeps is green now. LGTM. Thanks for knocking out a bunch of these ignores!

@pllim pllim merged commit 678c5ed into spacetelescope:main Oct 27, 2023
12 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
no-changelog-entry-needed changelog bot directive testing trivial Only needs one approval instead of two
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants