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

PR: Replace now deprecated distutils with setuptools #199

Merged
merged 1 commit into from
Dec 24, 2021

Conversation

kumattau
Copy link
Contributor

@kumattau kumattau commented Dec 22, 2021

This PR is preparing for removal of distutils in the future.

ref #194 (comment)

Copy link
Member

@dalthviz dalthviz left a comment

Choose a reason for hiding this comment

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

Thanks @kumattau for the help! Left some questions/suggestions 👍

setupbase.py Outdated Show resolved Hide resolved
setupbase.py Outdated Show resolved Hide resolved
@kumattau
Copy link
Contributor Author

I will check setuptools minimum version and remove distutils setup.py and setupbase.py.

I checked setuptools minimum version.
The following functions and classes are available in at least version 0.0.1 (in 2004) or later.
So, I think the minimum version is not required and distutils can be removed simply.

  • setuptools.setup
  • setuptools.command.install
  • setuptools.Command

On a related topic, "setup.py install" has been deprecated in setuptools 58.3.0 (05 Nov 2021).

pypa/setuptools@fc5c308#diff-d9f946e5d366d670898f9cfaa80a8f892853686b6533a07c40ba9e9d4ee0e862

I think it is better that we think separately "setup.py install" in another ticket if needs

@dalthviz
Copy link
Member

dalthviz commented Dec 24, 2021

I checked setuptools minimum version.
The following functions and classes are available in at least version 0.0.1 (in 2004) or later.
So, I think the minimum version is not required and distutils can be removed simply.

setuptools.setup
setuptools.command.install
setuptools.Command

Awesome @kumattau ! Lets drop the distutils fallback here then and add setuptools as a dependency

On a related topic, "setup.py install" has been deprecated in setuptools 58.3.0 (05 Nov 2021).

pypa/setuptools@fc5c308#diff-d9f946e5d366d670898f9cfaa80a8f892853686b6533a07c40ba9e9d4ee0e862

I think it is better that we think separately "setup.py install" in another ticket if needs

Sure, please open a new issue to give a follow-up to that 👍

@kumattau
Copy link
Contributor Author

kumattau commented Dec 24, 2021

Awesome @kumattau ! Lets drop the distutils fallback here then and add setuptools as a dependency

Specifically, do I need to add setuptools to environment.yml?

diff --git a/requirements/environment.yml b/requirements/environment.yml
index 978b804..ab4ed8a 100644
--- a/requirements/environment.yml
+++ b/requirements/environment.yml
@@ -8,3 +8,4 @@ dependencies:
 - pytest-cov
 - pytest-qt
 - qtpy
+- setuptools

I think this is not required because conda installs setuptools as default.

$ conda create -n py36 python=3.6
Collecting package metadata (current_repodata.json): done
Solving environment: done


==> WARNING: A newer version of conda exists. <==
  current version: 4.10.3
  latest version: 4.11.0

Please update conda by running

    $ conda update -n base -c defaults conda



## Package Plan ##

  environment location: /home/miniconda3/envs/py36

  added / updated specs:
    - python=3.6


The following NEW packages will be INSTALLED:

  _libgcc_mutex      pkgs/main/linux-64::_libgcc_mutex-0.1-main
  _openmp_mutex      pkgs/main/linux-64::_openmp_mutex-4.5-1_gnu
  ca-certificates    pkgs/main/linux-64::ca-certificates-2021.10.26-h06a4308_2
  certifi            pkgs/main/noarch::certifi-2020.6.20-pyhd3eb1b0_3
  ld_impl_linux-64   pkgs/main/linux-64::ld_impl_linux-64-2.35.1-h7274673_9
  libffi             pkgs/main/linux-64::libffi-3.3-he6710b0_2
  libgcc-ng          pkgs/main/linux-64::libgcc-ng-9.3.0-h5101ec6_17
  libgomp            pkgs/main/linux-64::libgomp-9.3.0-h5101ec6_17
  libstdcxx-ng       pkgs/main/linux-64::libstdcxx-ng-9.3.0-hd4cf53a_17
  ncurses            pkgs/main/linux-64::ncurses-6.3-h7f8727e_2
  openssl            pkgs/main/linux-64::openssl-1.1.1l-h7f8727e_0
  pip                pkgs/main/linux-64::pip-21.2.2-py36h06a4308_0
  python             pkgs/main/linux-64::python-3.6.13-h12debd9_1
  readline           pkgs/main/linux-64::readline-8.1-h27cfd23_0
  setuptools         pkgs/main/linux-64::setuptools-58.0.4-py36h06a4308_0
  sqlite             pkgs/main/linux-64::sqlite-3.36.0-hc218d9a_0
  tk                 pkgs/main/linux-64::tk-8.6.11-h1ccaba5_0
  wheel              pkgs/main/noarch::wheel-0.37.0-pyhd3eb1b0_1
  xz                 pkgs/main/linux-64::xz-5.2.5-h7b6447c_0
  zlib               pkgs/main/linux-64::zlib-1.2.11-h7f8727e_4


Proceed ([y]/n)? y

Preparing transaction: done
Verifying transaction: done
Executing transaction: done
#
# To activate this environment, use
#
#     $ conda activate py36
#
# To deactivate an active environment, use
#
#     $ conda deactivate

@kumattau kumattau changed the title PR: Prioritize setuptools over now deprecated distutils PR: Replace now deprecated distutils with setuptools Dec 24, 2021
@dalthviz
Copy link
Member

Specifically, do I need to add setuptools to environment.yml?

diff --git a/requirements/environment.yml b/requirements/environment.yml
index 978b804..ab4ed8a 100644
--- a/requirements/environment.yml
+++ b/requirements/environment.yml
@@ -8,3 +8,4 @@ dependencies:

  • pytest-cov
  • pytest-qt
  • qtpy
    +- setuptools

I think this is not required because conda installs setuptools as default.

Thinking about that maybe is not necessary since we don't really need a minimum version that supports the Command class so I think this LGTM as it is. Thanks @kumattau !

@dalthviz dalthviz merged commit a31fa28 into spyder-ide:master Dec 24, 2021
@kumattau kumattau deleted the prioritize-setuptools branch December 24, 2021 15:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants