TST: fix a test for compatibility with Python 3.13#16659
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 |
70023af to
e604ec4
Compare
| # model is not None | ||
| param._model_required = False | ||
| model = mk.MagicMock() | ||
| with mk.patch.object(functools, "partial", autospec=True) as mkPartial: |
There was a problem hiding this comment.
I don't grok what the original code was trying to do, being unfamiliar with so deep within modeling internals. This was added by @WilliamJamieson in #12232 so hopefully he can review? 🙏
There was a problem hiding this comment.
Would it be easier to use pytest.monkeypatch instead of unittest.mock?
There was a problem hiding this comment.
No, it is simply not safe to patch this function at all (see linked issue at CPython).
My understanding is that MagicMock was merely a convenient way to implement comparison in very few lines.
942538e to
e1e2f8f
Compare
|
image test failure is clearly not correlated, and addressed in #16722 |
e1e2f8f to
c2800e2
Compare
|
rebased to check Python 3.13.0b4 |
c2800e2 to
9fd25c0
Compare
|
This is now based on top of #16596 so that PR can be merged first. Switching this one to draft for now. |
9fd25c0 to
cce2172
Compare
|
If you rebase again, only the modeling test should be in diff here. And please remember to remove the xfail. Thanks! |
cce2172 to
cbff431
Compare
|
whoops, I thought I did already, but I clearly made a mistake in my previous rebase. It's fixed now, thanks for catching this ! |
cbff431 to
d6a9ac0
Compare
nden
left a comment
There was a problem hiding this comment.
This looks like a better solution.
…659-on-v6.1.x Backport PR #16659 on branch v6.1.x (TST: fix a test for compatibility with Python 3.13)
Description
This pull request is to address the only known incompatibility with Python 3.13 in our test suite (see #16600)
xref upstream issue: python/cpython#121257 (in particular python/cpython#121257 (comment))
The problematic (long) test was introduced in a large PR (#12232) so I can only guess that mocking was used only as a convenience and expanding the assertion is really equivalent (just more verbose and incidentally, more reliable).