Skip to content

Update UUID is_safe default value #9929

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

Closed
wants to merge 1 commit into from

Conversation

aminalaee
Copy link
Contributor

The UUID class is_safe parameter doesn't reflect the default value properly.
This is available in Python versions so I don't think this is a limitation, but a mismatch.

@AlexWaygood As it's related to #9606 can you please check?

@aminalaee
Copy link
Contributor Author

aminalaee commented Mar 23, 2023

I've read the Y011 description and my case was exactly using the IDE not the type checker.
Since this specific field has a literal value of None, there's no way to do this, right?

@github-actions
Copy link
Contributor

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

@AlexWaygood
Copy link
Member

This isn't allowed under our current rules. Currently we only permit int, float, complex, bytes, str, bool, None, ..., or simple container literals as default values. uuid.SafeUUID.unknown is an enum member, so doesn't fall into any of the above categories:

>>> import uuid
>>> uuid.SafeUUID.unknown
<SafeUUID.unknown: None>
>>> type(uuid.SafeUUID.unknown)
<enum 'SafeUUID'>

I'm open to a debate on whether we should change our current rules, but we should have that discussion in an issue :) There are various things to think through. For example, stubtest currently verifies in CI if a default value is correct, providing it falls into one of the above categories. But for an enum, it wouldn't provide any verification at all; it would just give up. So that would be a hole in our tests, which we rely on extensively to ensure we have accurate stubs :)

@aminalaee aminalaee deleted the update-uuid-is-safe-default branch March 23, 2023 16:25
@aminalaee
Copy link
Contributor Author

Thank you for the reply, that's what I thought too after seeing the flake8 failure.
For the sake of clarity I will open an issue for this to see pros and cons and see if any action is needed :)

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.

2 participants