faker icon indicating copy to clipboard operation
faker copied to clipboard

Type annotations on `md5`, `sha1`, and `sha256` should be overloaded

Open samueljsb opened this issue 1 year ago • 6 comments

  • Faker version: 25.0.1
  • OS: all

The docstrings for these functions state:

If raw_output is False (default), a hexadecimal string representation of the MD5 hash will be returned. If True, a bytes object representation will be returned instead.

But the type annotations do not reflect this:

$ cat t.py
import faker

fake = faker.Faker()

x = fake.md5(raw_output=False)
reveal_type(x)

y = fake.md5(raw_output=True)
reveal_type(y)

$ mypy t.py
$ mypy t.py
t.py:6: note: Revealed type is "Union[builtins.bytes, builtins.str]"
t.py:9: note: Revealed type is "Union[builtins.bytes, builtins.str]"
Success: no issues found in 1 source file

The revealed types ought to be builtins.str and builtins.bytes respectively.

I believe this can be achieved by overloading the type annotations on those methods (using typing.overload.

I would be happy to open a PR to make those changes, if that would be welcome?

samueljsb avatar May 13 '24 12:05 samueljsb

Thank you so much, that would be wonderful!

fcurella avatar May 13 '24 15:05 fcurella

I've tried to do this but I'm having trouble with the stub files. They don't seem to match the type annotations I put in the actual Python modules (even after running the stub generation script) and manually editing them gets undone when I run the linters.

EDIT: the stub generation doesn't appear to be running in CI, so that isn't a blocker, but it does mean we run the risk of this regressing.

samueljsb avatar May 15 '24 15:05 samueljsb

If I'm still in time to catch this train - the same applies to uuid4(), which has a return type of str | bytes | UUID, and the actual type returned is similarly controlled by a function parameter. https://github.com/joke2k/faker/issues/2042

steve-mavens avatar May 17 '24 15:05 steve-mavens

the generate_stubs.py script will need to be updated to detect overloads, but typing.get_overloads() is only available in Python 3.11, and we need to support at least 3.8.

fcurella avatar Jun 03 '24 14:06 fcurella

It's in typing_extensions if that's any help for pre-3.11. The trouble is it that pre-3.11 it only detects overloads defined using typing_extensions.overload, not typing.overload.

steve-mavens avatar Jun 03 '24 14:06 steve-mavens

I dont see any issues with falling back to typing_extension on <3.11. Do you want to try to update generate_stubs.py to use get_overloads()?

fcurella avatar Jun 03 '24 17:06 fcurella

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

github-actions[bot] avatar Sep 02 '24 01:09 github-actions[bot]

This issue was closed because it has been inactive for 14 days since being marked as stale.

github-actions[bot] avatar Sep 16 '24 02:09 github-actions[bot]