chore: add types #225

miketheman · 2022-03-10T01:32:45Z

There's a lot of commits here for a relatively small amount of changes, and it can seem overwhelming at first.

I tried to keep each commit well-scoped, and describe the journey on adding mypy as a testing strategy, correcting the issues surfaced during the initial (non-strict) runs, and then applying stricter rules, until it's finally a fully-typed library.

(I snuck in a pytest warning removal while I was at it.)

Completing this work, and releasing a version should allow us to remove a line in the warehouse config.

Let me know what y'all think!

Resolves #166

Scaffolding in the testing, and will not complete succssfully yet. Will be followed by code changes to pass. Signed-off-by: Mike Fiedler <miketheman@gmail.com>

Surfaced via `mypy`, in that an empty dict could not infer what types could be there. Instead of importing `typing` and annotating the empty Dict, I opted to ignore the line, as we do not expect to populate the dict at all, and are using it to **prevent** additions to the value. Signed-off-by: Mike Fiedler <miketheman@gmail.com>

Surfaced via `mypy`, recommended adding a type to the empty list. The list was originally empty back in 0.1.0. Instead of adding a type, remove the constant, and the code that uses it from `clean()` - as it was partially reverted in pypa#121. The default `ALLOWED_STYLES` in the underlying library is `[]`. Related: pypa#114 (comment) Signed-off-by: Mike Fiedler <miketheman@gmail.com>

Surfaced via `mypy`, in that the `html.parser` module does not have a direct implementation of `unescape`. Refs: https://docs.python.org/3.6/library/html.html#html.unescape In pypa#192 support for Python 2.7 was removed, the import path changed. This works due to imports placing the imported code in the local scope. If the `html.parser` module ever stopped importing `unescape`, this import would break as a result. Signed-off-by: Mike Fiedler <miketheman@gmail.com>

Currently emits a warning: PytestRemovedIn8Warning: The --strict option is deprecated, use --strict-markers instead. Signed-off-by: Mike Fiedler <miketheman@gmail.com>

Surfaced by running mypy in strict mode, and added types where relevant. Signed-off-by: Mike Fiedler <miketheman@gmail.com>

Signed-off-by: Mike Fiedler <miketheman@gmail.com>

The types-docutils hints are still incomplete, good progress is being made. See: python/typeshed#7256 I've had to use an ignore on the class inheritance, and a couple of `typing.Any` annotations until that package implements more type hints. Signed-off-by: Mike Fiedler <miketheman@gmail.com>

`mypy` strict mode is having a hard time with the `distutils` imports, since they are wrapped in `setuptools` right now as a private package. This pacakge's distutils integration will need to be reworked anyhow. Left a comment with details at the top of the file. Signed-off-by: Mike Fiedler <miketheman@gmail.com>

Prevent new things from creeping in during development. Signed-off-by: Mike Fiedler <miketheman@gmail.com>

Include a blank `py.typed` file in the package to inform `mypy` that there's types to be found in this package. Signed-off-by: Mike Fiedler <miketheman@gmail.com>

di

Thank you! LGTM, I'll give this ~24 hours for others to review before merging.

bhrutledge

Thanks for doing this! At @di's suggestion, I reviewed it based on my experience in Twine. I've got some questions/suggestions about style, but I don't think there's anything that should be considered a blocker.

I confirmed that using this branch allows Twine to remove its mypy ignore for this package.

readme_renderer/markdown.py

readme_renderer/rst.py

readme_renderer/clean.py

readme_renderer/rst.py

tox.ini

pyproject.toml

miketheman · 2022-03-10T12:23:35Z

@bhrutledge Thanks for the review!

Allows other tools to ebenfit from a consistent configuration. Signed-off-by: Mike Fiedler <miketheman@gmail.com>

Signed-off-by: Mike Fiedler <miketheman@gmail.com>

pyproject.toml

readme_renderer/clean.py

readme_renderer/rst.py

Signed-off-by: Mike Fiedler <miketheman@gmail.com>

miketheman · 2022-03-11T12:57:55Z

@di would appreciate the CI-approval to get the tests running to confirm.

bhrutledge · 2022-03-11T13:47:03Z

readme_renderer/markdown.py

+    # The renderer is a lambda function, and mypy fails lambdas right now.
+    rendered = renderer(raw)  # type: ignore


Did you try to work around this? I wonder if Callable and/or cast would help. Or, maybe variants could be updated to use explicit private functions with annotations, instead of lambdas.

I did not try to modify the code to make the type annotations work - that felt counter to the effort at hand. mypy struggles with lambdas - see open issue: python/mypy#4226

It'd be awesome if you wanted to take on that as a follow-up once this is merged to remove the ignore!

bhrutledge

Thanks again for taking my feedback; it was good exercise for me. :)

di · 2022-03-11T20:12:38Z

Released in https://pypi.org/project/readme-renderer/34.0/.

With the release of `readme_renderer` 35.0, we no longer need to exclude the library from type evaluation. Refs: pypa/readme_renderer#225 Refs: pypa/readme_renderer#228 Signed-off-by: Mike Fiedler <miketheman@gmail.com>

With the release of `readme_renderer` 35.0, we no longer need to exclude the library from type evaluation. Refs: pypa/readme_renderer#225 Refs: pypa/readme_renderer#228 Signed-off-by: Mike Fiedler <miketheman@gmail.com> Co-authored-by: Dustin Ingram <di@users.noreply.github.com>

As of pypa#225, we now exposed type information to users of the library. Let's also show it via classifier! Signed-off-by: Mike Fiedler <miketheman@gmail.com>

miketheman added 12 commits March 9, 2022 23:38

chore: add mypy testing

e2096de

Scaffolding in the testing, and will not complete succssfully yet. Will be followed by code changes to pass. Signed-off-by: Mike Fiedler <miketheman@gmail.com>

chore: update pytest markers cli flag

144692e

Currently emits a warning: PytestRemovedIn8Warning: The --strict option is deprecated, use --strict-markers instead. Signed-off-by: Mike Fiedler <miketheman@gmail.com>

chore(types): add types to clean module

3224f56

Surfaced by running mypy in strict mode, and added types where relevant. Signed-off-by: Mike Fiedler <miketheman@gmail.com>

chore(types): add types to txt module

8eb342f

Signed-off-by: Mike Fiedler <miketheman@gmail.com>

chore(types): add types to markdown module

c959bfd

Signed-off-by: Mike Fiedler <miketheman@gmail.com>

test: use strict mode for mypy

51fb70d

Prevent new things from creeping in during development. Signed-off-by: Mike Fiedler <miketheman@gmail.com>

chore: tell the world we've got types

c8c444a

Include a blank `py.typed` file in the package to inform `mypy` that there's types to be found in this package. Signed-off-by: Mike Fiedler <miketheman@gmail.com>

di mentioned this pull request Mar 10, 2022

Add type annotations #166

Closed

di approved these changes Mar 10, 2022

View reviewed changes

bhrutledge reviewed Mar 10, 2022

View reviewed changes

miketheman added 5 commits March 10, 2022 13:35

chore: move strict flag to config

66f3ec3

Allows other tools to ebenfit from a consistent configuration. Signed-off-by: Mike Fiedler <miketheman@gmail.com>

refactor: change imports to be consistent with others

77ffeff

Signed-off-by: Mike Fiedler <miketheman@gmail.com>

docs: add inline details for specific ignores

887cdc3

Signed-off-by: Mike Fiedler <miketheman@gmail.com>

lint: apply more specific type

a8203b2

Signed-off-by: Mike Fiedler <miketheman@gmail.com>

docs: add comment to why source is Any

4be323a

Signed-off-by: Mike Fiedler <miketheman@gmail.com>

miketheman requested review from bhrutledge and di March 10, 2022 14:05

bhrutledge reviewed Mar 10, 2022

View reviewed changes

pyproject.toml Show resolved Hide resolved

pyproject.toml Show resolved Hide resolved

readme_renderer/clean.py Outdated Show resolved Hide resolved

readme_renderer/rst.py Outdated Show resolved Hide resolved

lint: replace typing imports with relative ones

710c2e7

Signed-off-by: Mike Fiedler <miketheman@gmail.com>

miketheman force-pushed the miketheman/mypy branch from 2d85807 to 710c2e7 Compare March 11, 2022 12:56

miketheman requested a review from bhrutledge March 11, 2022 12:57

bhrutledge reviewed Mar 11, 2022

View reviewed changes

bhrutledge approved these changes Mar 11, 2022

View reviewed changes

di merged commit 5d721e1 into pypa:main Mar 11, 2022

miketheman deleted the miketheman/mypy branch March 11, 2022 22:07

bhrutledge mentioned this pull request Mar 12, 2022

Fix renderer ignore #230

Merged

miketheman mentioned this pull request Jun 12, 2022

chore(types): remove excluded library pypi/warehouse#11570

Merged

miketheman mentioned this pull request Aug 19, 2022

chore: advertise typing status in classifier #254

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

chore: add types #225

chore: add types #225

miketheman commented Mar 10, 2022

di left a comment

bhrutledge left a comment

miketheman commented Mar 10, 2022

miketheman commented Mar 11, 2022

bhrutledge Mar 11, 2022

miketheman Mar 11, 2022

bhrutledge left a comment

di commented Mar 11, 2022

		# The renderer is a lambda function, and mypy fails lambdas right now.
		rendered = renderer(raw) # type: ignore

chore: add types #225

chore: add types #225

Conversation

miketheman commented Mar 10, 2022

di left a comment

Choose a reason for hiding this comment

bhrutledge left a comment

Choose a reason for hiding this comment

miketheman commented Mar 10, 2022

miketheman commented Mar 11, 2022

bhrutledge Mar 11, 2022

Choose a reason for hiding this comment

miketheman Mar 11, 2022

Choose a reason for hiding this comment

bhrutledge left a comment

Choose a reason for hiding this comment

di commented Mar 11, 2022