Conversation
setup.cfg
Outdated
| @@ -1,2 +1,32 @@ | |||
| [bdist_wheel] | |||
| universal=0 | |||
| [metadata] | |||
There was a problem hiding this comment.
Might as well put this config in pyproject.toml?
There was a problem hiding this comment.
I don't have python 3.5 installed, but I suspect it won't work with it. I'll try getting py3.5 and check it though.
There was a problem hiding this comment.
We can drop 3.5, 3.6 and 3.7 support
There was a problem hiding this comment.
@hauntsaninja great, that might help. I'll send a PR to do this first.
Adding pyproject.toml allows the package to be installed using PEP517 instead of legacy `setup.py install` method. At least pip says so in a warning when installing without the `wheel` package available. Change setup.py to setup.cfg (using a script[0]) because setup.py is out of fashion these days. Remove MANIFEST.ini since LICENSE is implied[1]. [0] https://github.com/gvalkov/setuptools-py2cfg [1] https://setuptools.pypa.io/en/latest/references/keywords.html#keyword-license-files
fb504bf to
780aa14
Compare
Supported since setuptools 61.0.0, more standard.
0601a1f to
aad9410
Compare
|
Changed from setup.cfg to pyproject.toml, seems to work. Added it in a separate commit just in case.
|
nicoonoclaste
left a comment
There was a problem hiding this comment.
I'm absolutely not a mypy dev, but I noticed this PR while reporting an issue so have a drive-by review ❤️
This all seems good to me, and I only found a minor issue that (I believe) is present in the current version as well.
Thanks a bunch for tidying up the packaging :)
| name = "mypy_extensions" | ||
| version = "1.0.0-dev" | ||
| description = "Type system extensions for programs checked with the mypy type checker." | ||
| readme = "README.md" |
There was a problem hiding this comment.
This should specify the content type, as setuptools otherwise assumes it's ReStructured Text:
| readme = "README.md" | |
| readme = { file = "README.md", content-type = "text/markdown; charset=UTF-8" } |
There was a problem hiding this comment.
That's not necessary. Setuptools can detect that it's a markdown file and adds Description-Content-Type: text/markdown to the core metadata.
There was a problem hiding this comment.
One final change can be done to eliminate theActually seems to work with the module as well, so just removed it.tool.setuptoolspy-modulessection by moving from a single modulemypy_extensions.pyto a package, then setuptools can auto-discover it.
I'd keep it. Explicit is often better than implicit. I.e. add
[tool.setuptools]
py-modules = ["mypy_extensions"]--
There are also some conflicts which need to be resolved.
| name = "mypy_extensions" | ||
| version = "1.0.0-dev" | ||
| description = "Type system extensions for programs checked with the mypy type checker." | ||
| readme = "README.md" |
There was a problem hiding this comment.
That's not necessary. Setuptools can detect that it's a markdown file and adds Description-Content-Type: text/markdown to the core metadata.
| version = "1.0.0-dev" | ||
| description = "Type system extensions for programs checked with the mypy type checker." | ||
| readme = "README.md" | ||
| license = {file = "LICENSE"} |
There was a problem hiding this comment.
| license = {file = "LICENSE"} | |
| license = {text = "MIT"} |
I'd suggest to use the SPDX license string here instead. The license file is already included by setuptools.
| "Programming Language :: Python :: 3.11", | ||
| "Topic :: Software Development", |
There was a problem hiding this comment.
| "Programming Language :: Python :: 3.11", | |
| "Topic :: Software Development", | |
| "Programming Language :: Python :: 3.11", | |
| "Programming Language :: Python :: 3.12", | |
| "Topic :: Software Development", |
Support for 3.12 was declared in 9dd6d98
|
Thank you! |
Adding pyproject.toml allows the package to be installed using PEP517 instead of legacy
setup.py installmethod. At least pip says so in a warning when installing without thewheelpackage available.Change setup.py to setup.cfg (using a script[0]) because setup.py is out of fashion these days.
Remove MANIFEST.ini since LICENSE is implied[1].
[0] https://github.com/gvalkov/setuptools-py2cfg
[1] https://setuptools.pypa.io/en/latest/references/keywords.html#keyword-license-files