Skip to content
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.

Sign up for GitHub

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

Jump to bottom

uuid4() - become unusuable for safe typing without casting or ignoring #2042

Open
sshishov opened this issue May 5, 2024 · 6 comments
Open

uuid4() - become unusuable for safe typing without casting or ignoring #2042

sshishov opened this issue May 5, 2024 · 6 comments

Comments

@sshishov
Copy link
Contributor

sshishov commented May 5, 2024

  • Faker version: 25.0.1
  • OS: MacOS 14.4.1 (23E224)

When using fake.uuid4(), the returning type is bytes | str | UUID which is impossible to safely use later on in the code.
I guess one way to solve it to use literals instead of cast_to callable, as there are limited ways to cast it to. Only 3 types, therefore better to use Literal as cast_to and provide @typing.overload to properly infer return type from the input param. Also by default it should return str

Steps to reproduce

Nypy will produce error as for type checker the value can be either bytes or str or UUID which cannot be joined together. But in runtime here will no be any errors

value1 = fake.uuid4()
value2 = fake.uuid4()

joined = ','.join([value1, value2])  # <-- errors will be here

Expected behavior

No typing errors should be observed

Actual behavior

Using fake.uuid4() without typing.cast or type: ignore appears impossible after adding the types.
@sshishov sshishov changed the title Usage of uuid4 become unusuable for safe typing without casting or ignoring uuid4() - become unusuable for safe typing without casting or ignoring May 5, 2024
@stefan6419846
Copy link
Contributor

Feel free to provide a corresponding PR which improves this.

@sshishov
Copy link
Contributor Author

sshishov commented May 5, 2024

@stefan6419846 it will require breaking changes, is it okay? I definitely can provide PR for this one

@fcurella
Copy link
Collaborator

fcurella commented May 8, 2024

Can't this be fixed by using some kind of TypeVar in the type hint of uuid4?

@steve-mavens steve-mavens mentioned this issue May 17, 2024
Closed
@steve-mavens
Copy link

steve-mavens commented May 17, 2024 •
edited
Loading

Based on the type in the docs:

def uuid4(cast_to: ~typing.Callable[[~uuid.UUID], str] | ~typing.Callable[[~uuid.UUID], bytes] | None = <class 'str'>) → bytes | str | UUID

Like fcurella says, I think it should be possible to overload it like:

T = TypeVar('T', str, bytes)

@overload
def uuid4() -> str: ...
@overload
def uuid4(cast_to: Callable[[UUID], T]) -> T: ...
@overload
def uuid4(cast_to: None) -> UUID: ...

Failing that, get rid of T by explicitly listing both possibilities:

@overload
def uuid4() -> str: ...
@overload
def uuid4(cast_to: Callable[[UUID], str]) -> str: ...
@overload
def uuid4(cast_to: Callable[[UUID], bytes]) -> bytes: ...
@overload
def uuid4(cast_to: None) -> UUID: ...

I don't think that should be a breaking change.

Or potentially you could make the function more general, allowing cast_to to convert to any type at all instead of just str or bytes:

T = TypeVar('T')

@overload
def uuid4() -> str: ...
@overload
def uuid4(cast_to: Callable[[UUID], T]) -> T: ...
@overload
def uuid4(cast_to: None) -> UUID: ...

@github-actions GitHub Actions
Copy link

This issue is stale because it has been open for 30 days with no activity.

@github-actions github-actions bot added the stale label Aug 16, 2024
@sshishov
Copy link
Contributor Author

Let me guys try to create MR for this and we can see how far forward we can go with this...

@github-actions github-actions bot removed the stale label Aug 21, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
4 participants
@fcurella @sshishov @steve-mavens @stefan6419846