-
Notifications
You must be signed in to change notification settings - Fork 281
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
Catch Py312+ warning when using ratarmount #4819
Catch Py312+ warning when using ratarmount #4819
Conversation
@yt-fido test this please |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We should probably not except the deprecation warning to pop up, as this will fail again if/when ratarmount fixes it in the future.
If the fix is just to ignore the warning for now, it should be done in conftest.py
I think we also need to report the problem upstream and link the issue in a comment next the the warning filter |
dea5e73
to
3524c6b
Compare
conftest.py
Outdated
# On Python 3.12+, there is a deprecation warning when calling os.fork() | ||
# in a multi-threaded process |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this comment should be a condition. Something like
if find_spec("ratarmount") is not None and sys.version_info >= (3, 12)
:
3524c6b
to
daa6be5
Compare
I have now caught the warning in the config file. Note however that the warning is not a ratarmount issue but a yt one. It is caused by the fact that ratarmount is called in a subprocess (see comment: https://github.com/cphyc/yt/blob/3524c6b305fcd710aa0155dd30e9e4ae39085a5d/yt/loaders.py#L1792-L1795). In practice, this is only a problem if |
Dear me. Can't wait to see how this behaves in no-gil Python. Anyway, thank you for addressing this ! |
PR Summary
Since Python 3.12,
os.fork()
throws a deprecation warning when called from multiple threads.This causes the tests relying on ratarmount (to mount archives) to fail. In this PR, I superficially address the issue by catching the warning on the test side.
The long-term solution would be to update
ratarmount
to a more recent version.