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.

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

Update mypy Workaround if It Is No Longer Needed #3

Open
4 tasks
mcdonnnj opened this issue Feb 10, 2021 · 0 comments
Open
4 tasks

Update mypy Workaround if It Is No Longer Needed #3

mcdonnnj opened this issue Feb 10, 2021 · 0 comments
Labels
improvement This issue or pull request will add new or improve existing functionality need info This issue or pull request requires further information

Comments

@mcdonnnj
Copy link
Member

mcdonnnj commented Feb 10, 2021

💡 Summary

We currently work around a perceived issue with type hinting with mypy in:

# mypy relies on typeshed (https://github.com/python/typeshed) for
# stdlib type hinting, but it does not have the correct type hints for
# hashlib.new(). The PR I submitted to fix them
# (https://github.com/python/typeshed/pull/4973) was approved, but I
# am not sure if mypy will still have issues with the usage of this
# keyword in non Python 3.9 (when the usedforsecurity kwarg was added)
# environments. I believe the earliest I can test this will be in mypy
# v0.900, and I have made
# https://github.com/cisagov/hash-http-content/issues/3 to document
# the status of this workaround.
# hasher = hashlib.new(hash_algorithm, usedforsecurity=False)
hasher = getattr(hashlib, "new")(hash_algorithm, usedforsecurity=False)

After a new version of mypy with type hint updates (possibly v0.900) is released, we should see if this workaround is still necessary to pass linting.

Motivation and context

Workarounds should only be used as long as there is something to work around.

Implementation notes

The preferred usage is commented out, so testing is simple as switching between the current and preferred usage.

Acceptance criteria

  • mypy hook passes with the preferred usage in place
  • Code is updated

or

  • mypy hook still fails with the preferred usage in place
  • Comment is updated to reflect the necessity of the workaround
@mcdonnnj mcdonnnj added improvement This issue or pull request will add new or improve existing functionality need info This issue or pull request requires further information labels Feb 10, 2021
mcdonnnj added a commit that referenced this issue Feb 11, 2023
Undo the `mypy` workaround described in #3 now that we are using a much
newer version.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
improvement This issue or pull request will add new or improve existing functionality need info This issue or pull request requires further information
Projects
None yet
Development

No branches or pull requests

1 participant