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

[setup] drop deprecated distutils usage #22531

Merged
merged 4 commits into from
Apr 3, 2023
Merged

Conversation

XuehaiPan
Copy link
Contributor

@XuehaiPan XuehaiPan commented Apr 3, 2023

What does this PR do?

Migrate setup script to pyproject.toml (PEP 517 – A build-system independent format for source trees).

Changes:

  • drop deprecated distutils usage
  • migrate setup script to pyproject.toml
  • migrate isort and pytest configs to pyproject.toml
  • migrate flake8 configs to .flake8 and remove setup.cfg file

Before submitting

  • This PR fixes a typo or improves the docs (you can dismiss the other checks if that's the case).
  • Did you read the contributor guideline,
    Pull Request section?
  • Was this discussed/approved via a Github issue or the forum? Please add a link
    to it if that's the case.
  • Did you make sure to update the documentation with your changes? Here are the
    documentation guidelines, and
    here are tips on formatting docstrings.
  • Did you write any new necessary tests?

Who can review?

Anyone in the community is free to review the PR once the tests have passed. Feel free to tag
members/contributors who may be interested in your PR.

@HuggingFaceDocBuilderDev
Copy link

HuggingFaceDocBuilderDev commented Apr 3, 2023

The documentation is not available anymore as the PR was closed or merged.

Copy link
Collaborator

@sgugger sgugger left a comment

Choose a reason for hiding this comment

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

Thanks for your PR, but we are very happy with the setup as it is.

@XuehaiPan XuehaiPan changed the title [setup] migrate setup script to pyproject.toml [setup] drop deprecated distutils usage Apr 3, 2023
@XuehaiPan
Copy link
Contributor Author

XuehaiPan commented Apr 3, 2023

but we are very happy with the setup as it is.

Thanks for the clarification. The pyprojct.toml format is the recommended packaging method in PEP 517 – A build-system independent format for source trees. I have reverted some of the commits but kept the first one. The distutils module is deprecated and will be removed in Python 3.12 (See also PEP 632 – Deprecate distutils module). In this PR, I changed distutils.core.Command to setuptools.Command.

setup.py Outdated Show resolved Hide resolved
setup.py Outdated Show resolved Hide resolved
setup.py Show resolved Hide resolved
Copy link
Collaborator

@sgugger sgugger left a comment

Choose a reason for hiding this comment

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

Thanks for your contribution!

@sgugger
Copy link
Collaborator

sgugger commented Apr 3, 2023

I think you can suggest the changes thate removed the setup.cfg in a separate PR, it's a good cleanup (but not relevant to this PR anymore)

For migrating the setup.py to the pyproject, let's see with @LysandreJik what he thinks. My first reaction is to keep what's been working for us all these years ;-)

@XuehaiPan
Copy link
Contributor Author

XuehaiPan commented Apr 3, 2023

Thanks for the clarification. Since we have already dropped Python 3.6 support, setuptools works very well with pyproject.toml based project. We can move the static parts in setup.py to pyproject.toml. Note that the optional dependencies are too dynamic, so we still need a setup.py file.

Most Python utilities support pyproject.toml configuration (black, isort, ruff, pytest, ...). And some do not even support other config files like setup.cfg (e.g., black). I think maintaining configurations in a single file is a good practice. If you decide to migrate to pyproject.toml, pin me if I can help.

@sgugger
Copy link
Collaborator

sgugger commented Apr 3, 2023

Yes, moving all configurations to the pyproject.toml is something we would like to clean up. If you want to contribute it, please open a PR :-) Note that we kept the isort and flake8 configurations for a bit after our migration to ruff, but they can now be completely removed, so it would just be pytest if I'm not mistaken.

@sgugger sgugger merged commit 80d1319 into huggingface:main Apr 3, 2023
@XuehaiPan XuehaiPan deleted the packaging branch April 4, 2023 06:48
raghavanone pushed a commit to raghavanone/transformers that referenced this pull request Apr 5, 2023
* [setup] drop deprecated `distutils` usage

* drop deprecated `distutils.util.strtobool` usage

* fix import order

* reformat docstring by `doc-builder`
novice03 pushed a commit to novice03/transformers that referenced this pull request Jun 23, 2023
* [setup] drop deprecated `distutils` usage

* drop deprecated `distutils.util.strtobool` usage

* fix import order

* reformat docstring by `doc-builder`
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.

3 participants