Updated packaging format #606
Conversation
qingyun-wu
left a comment
qingyun-wu
left a comment
There was a problem hiding this comment.
Does this new packaging format require changes on how to do editable installs? If so, can you please also update the contributing guide. Otherwise looks good to me.
Having removed setup.py to comply with modern Python packaging standards (see microsoft#606), a recent version of pip is required to install the package in editable mode.
|
@qingyun-wu thank you for catching this - installing in editable mode without |
| Operating System :: OS Independent | ||
|
|
||
| [options] | ||
| python_requires = >= 3.7 |
There was a problem hiding this comment.
Question: does this way of packaging support python 3.6? I wanted to allow installation in python 3.6 by changing >=3.7 into >=3.6.
There was a problem hiding this comment.
I am almost completely positive that there shouldn't be a problem but I can't seem to find anything definitive trying to look it up. Would you like for me to add Python 3.6 to the CI matrix?
There was a problem hiding this comment.
I knew that CI would not pass in Python 3.6 because an optional dependency (seqeval) doesn't support 3.6 and the test under nlp/ fails. One workaround is to skip those tests in Python 3.6.
There was a problem hiding this comment.
Then just in terms of replacing setup.py with setup.cfg there shouldn't be a problem, but perhaps it's worth opening a seperate issue for adding support for Python 3.6 and updating this only after the CI is set up.
There was a problem hiding this comment.
Could you test the installation at least? No need to test optional dependencies or any unittests.
There was a problem hiding this comment.
Does the same command work in a different version of Python?
There was a problem hiding this comment.
Yes, running pip install . from the root directory while a Python 3.9 virtual environment is activated works without a problem.
There was a problem hiding this comment.
Got it. Is there a way to allow Python 3.6 installation? Though we do not plan to support Python 3.6 in future, I hope to give Python 3.6 users a grace period to be able to install it before they finish the migration.
There was a problem hiding this comment.
[Sorry if you already read my previous comment, I had a mistake, this is the correct status]
I rebuilt the devcontainer with Python 3.6 and could verify the installation is working properly with the original setup.py but raising the same exception I was getting before for setup.cfg. I tried looking up possible causes but unfortunately haven't managed to figure this out so far. Seems like collecting requirements for the wheel build isn't succeeding, but nothing seems to resolve it. I tried adding a minimal setup.py but that didn't work either.
I guess the options are either to drop support for 3.6 or wait with this PR until that happens.
There was a problem hiding this comment.
Thanks. If there is no strong reason to change the packaging format now, perhaps we should wait with this PR. In the same time, let's watch the progress of users' migration.
Replaces `packages=setuptools.find_packages(include=["flaml*"])` in the original setup.py file.
Borda
left a comment
Borda
left a comment
There was a problem hiding this comment.
Would it be better rather use pyproject.toml as setup.cfg is quite outdated... see: https://www.reddit.com/r/learnpython/comments/yqq551/pyprojecttoml_setupcfg_setuppy_whats_the/
It's a good time to revisit this issue since py3.6 support has been dropped. If |
5 participants
Closes #605.