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.

Sign up for GitHub

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

Jump to bottom

Move builds to use matplotlib 3.5.x #2233

Merged
merged 6 commits into from
Oct 11, 2022
Merged

Move builds to use matplotlib 3.5.x #2233

merged 6 commits into from
Oct 11, 2022

Conversation

llimeht
Copy link
Contributor

@llimeht llimeht commented Oct 4, 2022

Updating matplotlib to version 3.5.x means that the conflicting pinning and bugs in setuptools for matplotlib vs setuptools vs numba is avoided.

This should fix #2222.

This PR necessarily includes the commits from #2216 and #2213.

@wpotrzebowski
Copy link
Contributor

I've just tried Mac installer from this PR and I am still getting error:
ModuleNotFoundError: No module named 'setuptools._distutils'

@llimeht
Copy link
Contributor Author

llimeht commented Oct 5, 2022

Three additional changes here to get this to work:

  • Added setuptools._distutils as a hidden import in the installer. This is not enough to fix old matplotlib but is enough for some of the smaller uses of distutils.
  • Stop using distutils in SasView itself - distutils is deprecated and will get removed completely from Python stdlib next year. Being a build tool not a runtime tool, distutils doesn't have much use inside SasView and the three uses were either legacy cruft or a no-op statement.
  • Added a logging stage to the CI to make it easier to see what versions of the modules are installed. Manually wading through the output of the pip install steps has been painful, particularly as later steps can undo version pins introduced in previous steps. Using pip freeze to generate a list of packages and versions is simple info to just always gather.

This PR now produces a working windows installer.

@wpotrzebowski
Copy link
Contributor

Mac installer works now!

@wpotrzebowski wpotrzebowski self-requested a review October 5, 2022 12:02
wpotrzebowski
wpotrzebowski reviewed Oct 5, 2022
View reviewed changes
Copy link
Contributor

@wpotrzebowski wpotrzebowski left a comment

Choose a reason for hiding this comment

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

There was a discussion some time ago about strict versions and I think we concluded that we want to keep them (at least that's what I remmber).

@wpotrzebowski
Copy link
Contributor

Can someone test Windows installer?

@llimeht
Copy link
Contributor Author

llimeht commented Oct 5, 2022

There was a discussion some time ago about strict versions and I think we concluded that we want to keep them (at least that's what I remmber).

Sticking to PEP440 versions is a good idea. Importing and running StrictVersion(foo) doesn't actually do anything, however. (Well, it would raise a ValueError if the version is bad, but that's a thing to check at build time not every time you start SasView).

With distutils going away, there's not actually a long term option to keep that call to StrictVersion there in any case.

@krzywon
Copy link
Contributor

krzywon commented Oct 6, 2022

Can someone test Windows installer?

I just tested the Windows installer and SasView launched as expected.

There was a discussion some time ago about strict versions and I think we concluded that we want to keep them (at least that's what I remmber).

Thre was a recent discussion in PR #2168, specifically this comment chain but I think there was mention of it again in that PR as well.

krzywon
krzywon approved these changes Oct 6, 2022
View reviewed changes
Copy link
Contributor

@krzywon krzywon left a comment

Choose a reason for hiding this comment

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

I'm pretty certain the changes in sasview.__init__ will conflict with the work done in #2168 and the changes to setup.py might as well. The work is compatible, but things were moved around in that PR, so @lucas-wilkins might need to integrate these changes in his branch before merging.

@wpotrzebowski
Copy link
Contributor

@lucas-wilkins did you have a chance to look at it?

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

@lucas-wilkins did you have a chance to look at it?

@wpotrzebowski Not this one, but its a continuation of #2213 right? I approved that one, and this appears to just add the updates to requirements.txt and another PR.

Regarding the StrictVersion thing, we can do proper versioning without calling the StrictVersion constructor in __init__.

@krzywon There are likely some conflicts, they'll just have to be sorted by whoever merges last.

@llimeht
Fix compatibility with matplotlib 3.5.x
e51f9e2 
This patch attempts to fix compatibility with matplotlib 3.5.x by updating
the code to avoid functions that were deprecated (in some cases for years)
and have now been removed.

Merging this patch at least allows the sasview GUI to start with matplotlib
3.5; additional places where code needs to be updated may well be uncovered
by automated or manual testing.

Closes: SasView#2170
@llimeht
Update setup.py for setuptools 65
8a9b638 
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.
@llimeht
Move to mpl 3.5.x in builds
5a38d31 
Unpinning matplotlib allows a newer matplotlib to be installed, saves it
being broken by setuptools, and should therefore unbreak the build.

Closes: SasView#2222
@llimeht
Force setuptools._distutils into pyinstaller
14f9918
@llimeht
Remove usage of distutils
a4f91cf 
The distutils module is deprecated and will soon disappear from Python. This patch removes
use of distutils from SasView; none of the uses had any functionality already.
@llimeht
Document all python modules in CI
7b5b48e 
Use pip to produce a list of installed packages and their versions to
make it easier to see what versions of packages have ended up installed
in the build environment amongst pinning from this project and its
dependencies.
@llimeht
Copy link
Contributor Author

llimeht commented Oct 10, 2022

Rebased onto current main and trivial conflict in .spec file (automatically) resolved.

@wpotrzebowski wpotrzebowski merged commit 8d1020a into SasView:main Oct 11, 2022
This was referenced Oct 11, 2022
Closed
Closed
@llimeht llimeht deleted the tmp/mpl35unpin branch October 11, 2022 23:17
@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
Reviewers

@wpotrzebowski wpotrzebowski wpotrzebowski left review comments

@krzywon krzywon krzywon approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Breaking change in setuptools 50+
5 participants
@llimeht @wpotrzebowski @krzywon @lucas-wilkins @butlerpd