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

Update setup.py for setuptools 65 #2216

Closed
wants to merge 1 commit into from
Closed

Conversation

llimeht
Copy link
Contributor

@llimeht llimeht commented Sep 23, 2022

setuptools complains loudly if distutils is also used, and exits with error in the 'docs' step since distutils.core.Command rather than setuptools.Command was used for that step:

$ python3 setup.py docs
…/sasview/setup.py:12: DeprecationWarning: The distutils package is deprecated and slated for removal in Python 3.12. Use setuptools or check PEP 632 for potential alternatives
  from distutils.core import Command
/usr/lib/python3/dist-packages/_distutils_hack/__init__.py:18: UserWarning: Distutils was imported before Setuptools, but importing Setuptools also replaces the `distutils` module in `sys.modules`. This may lead to undesirable behaviors or errors. To avoid these issues, avoid using distutils directly, ensure that setuptools is installed in the traditional way (e.g. not an editable install), and/or make sure that setuptools is always imported before distutils.
  warnings.warn(
/usr/lib/python3/dist-packages/_distutils_hack/__init__.py:33: UserWarning: Setuptools is replacing distutils.
  warnings.warn("Setuptools is replacing distutils.")
Traceback (most recent call last):
  File "…/sasview/setup.py", line 293, in <module>
    setup(
  File "/usr/lib/python3/dist-packages/setuptools/__init__.py", line 87, in setup
    return distutils.core.setup(**attrs)
  File "/usr/lib/python3/dist-packages/setuptools/_distutils/core.py", line 172, in setup
    ok = dist.parse_command_line()
  File "/usr/lib/python3/dist-packages/setuptools/_distutils/dist.py", line 479, in parse_command_line
    args = self._parse_command_opts(parser, args)
  File "/usr/lib/python3/dist-packages/setuptools/dist.py", line 1107, in _parse_command_opts
    nargs = _Distribution._parse_command_opts(self, parser, args)
  File "/usr/lib/python3/dist-packages/setuptools/_distutils/dist.py", line 545, in _parse_command_opts
    raise DistutilsClassError(
distutils.errors.DistutilsClassError: command class <class '__main__.BuildSphinxCommand'> must subclass Command

This patch is a minimalist "keep it working" effort. Alternatively, all the doc-building could be ripped out of setup.py, noting that CI does not use it but calls build_sphinx.py directly. (And a long term goal of setup.py with a PEP517 build configuration remains.)

@llimeht llimeht mentioned this pull request Sep 25, 2022
@butlerpd butlerpd added the Discuss At The Call Issues to be discussed at the fortnightly call label Sep 26, 2022
setuptools complains loudly if distutils is also used, and exits with error
in the 'docs' step since distutils.core.Command rather than setuptools.Command
was used for that step.
@wpotrzebowski
Copy link
Contributor

Similar to #2214 the error occurs at start up:

Traceback (most recent call last):
  File "sasview.py", line 13, in <module>
  File "PyInstaller/loader/pyimod02_importers.py", line 499, in exec_module
  File "sas/qtgui/MainWindow/MainWindow.py", line 6, in <module>
  File "PyInstaller/loader/pyimod02_importers.py", line 499, in exec_module
  File "sas/sasview/__init__.py", line 1, in <module>
    """Python bindings for 0MQ."""
  File "_distutils_hack/__init__.py", line 88, in create_module
    path_before = os.environ.get("PATH")
  File "importlib/__init__.py", line 127, in import_module
    
  File "PyInstaller/loader/pyimod02_importers.py", line 499, in exec_module
  File "setuptools/__init__.py", line 10, in <module>
    from contextlib import contextmanager
  File "_distutils_hack/__init__.py", line 88, in create_module
    path_before = os.environ.get("PATH")
  File "importlib/__init__.py", line 127, in import_module
    
ModuleNotFoundError: No module named 'setuptools._distutils'
[84382] Failed to execute script 'sasview' due to unhandled exception: No module named 'setuptools._distutils'
[84382] Traceback:
Traceback (most recent call last):
  File "sasview.py", line 13, in <module>
  File "PyInstaller/loader/pyimod02_importers.py", line 499, in exec_module
  File "sas/qtgui/MainWindow/MainWindow.py", line 6, in <module>
  File "PyInstaller/loader/pyimod02_importers.py", line 499, in exec_module
  File "sas/sasview/__init__.py", line 1, in <module>
  File "_distutils_hack/__init__.py", line 88, in create_module
  File "importlib/__init__.py", line 127, in import_module
  File "PyInstaller/loader/pyimod02_importers.py", line 499, in exec_module
  File "setuptools/__init__.py", line 10, in <module>
  File "_distutils_hack/__init__.py", line 88, in create_module
  File "importlib/__init__.py", line 127, in import_module
ModuleNotFoundError: No module named 'setuptools._distutils'

@llimeht
Copy link
Contributor Author

llimeht commented Sep 29, 2022

@wpotrzebowski, no, that's #2222 and unrelated to this PR. The solution is to either upgrade setuptools or upgrade matplotlib (or both).

Copy link
Member

@rozyczko rozyczko left a comment

Choose a reason for hiding this comment

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

Works fine with newest setuptools.
python setup.py docs run fine now (failing later with unrelated error).

@wpotrzebowski wpotrzebowski removed the Discuss At The Call Issues to be discussed at the fortnightly call label Oct 4, 2022
@wpotrzebowski
Copy link
Contributor

@wpotrzebowski, no, that's #2222 and unrelated to this PR. The solution is to either upgrade setuptools or upgrade matplotlib (or both).

Sorry if this naive question but shouldn't package upgrade package be done as a part of this PR (by modyfing GH actions yml files)?

@llimeht
Copy link
Contributor Author

llimeht commented Oct 4, 2022

Sorry if this naive question but shouldn't package upgrade package be done as a part of this PR (by modyfing GH actions yml files)?

Not a naive question at all!

This patch is needed for setuptools ≥ 65 but does not require setuptools ≥ 65. This patch is compatible with setuptools from a decade-ish ago, so it can go in now as a preparatory patch to save future failures (such as the ones we already have in Debian/Ubuntu where we have setuptools 65.3). Since this patch is about ensuring broad compatibility with lots of different versions of setuptools, there's no need in this PR to force any particular version of setuptools.

The error you reported is not an error with this patch but rather an existing bug with the particular combination of numba/setuptools/matpotlib/pyinstaller that sasview is forcing. That's described in lots more detail in the bug report #2222 and fixing that is a separate PR. That PR doesn't exist yet (and so all the windows installers remain broken…) because there needs to be an agreed approach first.

My approach to fixing #2222 would be to remove the constraint on the old and unsupported matplotlib 2.2.5 and switch to something that is then supported by pyinstaller and doesn't interact badly with setuptools. Updating to a newer matplotlib will undoubtedly result in new bugs appearing and so those need to be found and fixed. That bugfixing needs to happen eventually and now is a reasonable time to do it since we're reasonably early in the development cycle for the next release.

@wpotrzebowski
Copy link
Contributor

@llimeht thank you for the explanation! It seems that currently most of PRs artifacts (tested installers on Mac) are affected by this issue and therefore cannot be properly tested. I agree we need to fix it properly and it is good time to do it now.

@wpotrzebowski wpotrzebowski added the Discuss At The Call Issues to be discussed at the fortnightly call label Oct 10, 2022
@wpotrzebowski
Copy link
Contributor

Closing because changes were part of #2233

@llimeht llimeht deleted the tmp/setup branch October 11, 2022 23:19
@butlerpd butlerpd removed the Discuss At The Call Issues to be discussed at the fortnightly call label Aug 1, 2023
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.

4 participants