TST: simplify a warning-capturing test#18954
Conversation
|
Thank you for your contribution to Astropy! 🌌 This checklist is meant to remind the package maintainers who will review this pull request of some common things to look for.
|
|
👋 Thank you for your draft pull request! Do you know that you can use |
| self._from_dataframe( | ||
| df, backend, use_legacy_pandas_api, units={"x": u.m, "t": u.s, "y": u.m} | ||
| ) | ||
| assert len(record) == 1 |
There was a problem hiding this comment.
You might still want to keep the len check, to catch multiple same warnings being emitted when it is not supposed to?
There was a problem hiding this comment.
A pytest.raise context scoped with a sufficiently restrictive match argument, should already achieve the desired goal. The issue I encountered with this test was specifically that another warning is emitted from a transitive dependency (with sufficiently old versions of everything), and while I still need to handle that other warning (left as a follow up), it would be pretty fragile to modify the expected number of warnings dynamically, so I wanted to first make this incremental improvement
There was a problem hiding this comment.
match does not guard against duplicate warnings. I think removing len check here is not a one-to-one code replacement but I'll defer to @taldcroft .
There was a problem hiding this comment.
The code in question can only raise one matching warning, so I'm OK with this.
| self._from_dataframe( | ||
| df, backend, use_legacy_pandas_api, units={"x": u.m, "t": u.s, "y": u.m} | ||
| ) | ||
| assert len(record) == 1 |
There was a problem hiding this comment.
The code in question can only raise one matching warning, so I'm OK with this.
Description
I stumbled upon a failure mode of this test case while working on #18782, and it took me longer than it should have to figure out what was happening. This should be a simple refactor, but a QoL improvement for debugging.