Skip to content

Added reporting extras option to setup.py #11777

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

Merged
merged 5 commits into from
Jan 2, 2022
Merged

Added reporting extras option to setup.py #11777

merged 5 commits into from
Jan 2, 2022

Conversation

jamesbraza
Copy link
Contributor

Description

Today, I was looking at the Report generation docs, and noticed lxml is listed as an additional requirement. I thought it would be nice to add as an extra to mypy's setup.py.

This way, inside a requirements-dev.txt, someone can include mypy[reporting] and have lxml installed.

One thing to note: a comment in setup.py says this:

# When changing this, also update mypy-requirements.txt.

NOTE: I did not update mypy-requirements.txt, as I belive extra requirements don't belong there. This will be up to the PR reviewer if he/she wants to add lxml to mypy-requirements.txt.

NOTE 2: I did not pin lxml. I only tested this to work with lxml==4.7.1.

Test Plan

Here is how I verified my change works.

pip install -e "git+ssh://[email protected]/jamesbraza/mypy.git@reporting#egg=mypy"
pip list

outputs:

Package           Version
----------------- --------------------------------------------------
mypy              0.940+dev.c7ca8e3d2c5a957510e70163c8155f2cd9a3f017
mypy-extensions   0.4.3
pip               21.3.1
setuptools        56.0.0
tomli             2.0.0
typing_extensions 4.0.1

then

pip install -e "git+ssh://[email protected]/jamesbraza/mypy.git@reporting#egg=mypy[reporting]"
pip list

outputs:

Package           Version                                            Editable project location
----------------- -------------------------------------------------- ----------------------------------------
lxml              4.7.1
mypy              0.940+dev.c7ca8e3d2c5a957510e70163c8155f2cd9a3f017 /Users/james.braza/code/mypy/hi/src/mypy
mypy-extensions   0.4.3
pip               21.3.1
setuptools        56.0.0
tomli             2.0.0
typing_extensions 4.0.1

I also ran the tests as documented in CONTRIBUTING.md.

Copy link
Collaborator

@hauntsaninja hauntsaninja left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks, this is useful. Would you mind updating the documentation section you linked to mention the extra?

Also to bikeshed, I think I have a vague preference for calling this [reports] instead of the gerund, but happy with whatever you feel is best.

@jamesbraza
Copy link
Contributor Author

Would you mind updating the documentation section you linked to mention the extra?

Made the changes in the docs, let me know what you think of the wording.

preference for calling this [reports]

No problem, implemented.

@hauntsaninja hauntsaninja merged commit b0c4461 into python:master Jan 2, 2022
@hauntsaninja
Copy link
Collaborator

Thank you!

@jamesbraza jamesbraza deleted the reporting branch January 2, 2022 15:29
tushar-deepsource pushed a commit to DeepSourceCorp/mypy that referenced this pull request Jan 20, 2022
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