Skip to content

fix: Add missing multiprocessing/resource-tracker type hints #8405

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

Merged
merged 6 commits into from
Jul 27, 2022
Merged

fix: Add missing multiprocessing/resource-tracker type hints #8405

merged 6 commits into from
Jul 27, 2022

Conversation

kkirsche
Copy link
Contributor

X-Ref: #6799

This adds best-effort types to multiprocessing.resource_tracker to begin to address issue #6799

The source file can be viewed here:
https://github.com/python/cpython/blob/main/Lib/multiprocessing/resource_tracker.py

@github-actions

This comment has been minimized.

@kkirsche
Copy link
Contributor Author

It sounds like this is Python 3.8+ only from this error:

error: multiprocessing.resource_tracker failed to import, ModuleNotFoundError: No module named 'multiprocessing.resource_tracker'
Stub: at line 1
MypyFile:1(
  stdlib/multiprocessing/resource_tracker.pyi)
Runtime:
MISSING

Before making changes though, would someone be able to confirm the correct way to address this? I want to avoid unnecessary emails and whatnot as I try to work it out myself

@JelleZijlstra
Copy link
Member

You may need to add it to the VERSIONS file if it's new in 3.8.

@kkirsche
Copy link
Contributor Author

kkirsche commented Jul 26, 2022

@github-actions

This comment has been minimized.

1 similar comment
@github-actions

This comment has been minimized.

@kkirsche
Copy link
Contributor Author

Fixed via c74e59c, thank you for the feedback!

@github-actions

This comment has been minimized.

Comment on lines 10 to 11
def register(self, name: Sized, rtype: object) -> None: ...
def unregister(self, name: Sized, rtype: object) -> None: ...
Copy link
Member

@AlexWaygood AlexWaygood Jul 27, 2022

Choose a reason for hiding this comment

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

It's true that these methods can be passed literally any object without raising an exception, so your instinct here to annotate these parameters with object was good. However, I feel like it's unlikely that you're actually meant to be passing literally any object to these methods. I think it's more likely that you're meant to be passing some kind of "resource" to these methods (the "shared memory segments, semaphores, etc." referenced in the block comment at the top of the module). I don't know enough about multiprocessing to know what the true type is, so I'd rather we went for _typeshed.Incomplete for now, which serves as an explicit marker of "we're not quite sure what this type should be".

Suggested change
def register(self, name: Sized, rtype: object) -> None: ...
def unregister(self, name: Sized, rtype: object) -> None: ...
def register(self, name: Sized, rtype: Incomplete) -> None: ...
def unregister(self, name: Sized, rtype: Incomplete) -> None: ...

(You'll have to import Incomplete from _typeshed.)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

That makes sense, updated :) thanks for explaining that 👍

@github-actions
Copy link
Contributor

According to mypy_primer, this change has no effect on the checked open source code. 🤖🎉

@AlexWaygood AlexWaygood merged commit 5397d43 into python:master Jul 27, 2022
@kkirsche kkirsche deleted the multiprocessing/resource-tracker branch July 27, 2022 16:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants