prospector icon indicating copy to clipboard operation
prospector copied to clipboard

update some dependency pins so we can build with Python=3.12

Open valeriupredoi opened this issue 1 year ago • 14 comments

Description

Hey folks, My apologies for busing in - my area is usually restricted to the conda package maintenance :grin: I am attempting to build a new Prospector 1.10.3 build but with Python 3.12, and for that I had to adjust some deps pins, see https://github.com/conda-forge/prospector-feedstock/pull/44 - package builds well (albeit with pip check turned off since it barfed that deps in the conda meta file were not matching upstream, and rightly so). Please have a look at the new pins, when you get a bit of time.

~There is, however, a fairly big catch - this way with flake8>=6 Python 3.7 will not be supported anymore, as per https://github.com/landscapeio/prospector/pull/551 - I'd argue that 3.7 needs be retired since it's EOL, but that's not my place to decide it for Prospector. At any rate, if the new pins are good for you and agree with retiring 3.7 then maybe keep this open until 3.7 retirement happens.~

Cheers :beer:

~EDIT in order to be able to regenerate the poetry lock file correctly (poetry==1.7.1 from PyPI) I had to already pin the lowest supported version of Python to 3.8.1 - now, I am aware this not a decision I can take, I am simply fixing the mechanics of this PR, so please bear this in mind~

With the more relaxed pins suggested by @sbrunner below, there is no more need to unsupport Python 3.7 - the build with Python 3.12 works very well and the environment supports 3.7-3.12 https://github.com/conda-forge/prospector-feedstock/pull/44 (added 3.12 to the Github Actions tests too) :+1:

Related Issue

https://github.com/landscapeio/prospector/issues/644

Motivation and Context

allows for building the package with Python 3.12, hence supporting the new Python :snake:

How Has This Been Tested?

Tested in the feedstock, purely from a dependencies analysis and pkg build point of view, no other code functionality test

Types of changes

  • ~[ ] Bug fix (non-breaking change which fixes an issue)~
  • ~[ ] New feature (non-breaking change which adds functionality)~
  • [x] Breaking change (fix or feature that would cause existing functionality to change): turns off support for Python 3.7

Checklist:

  • [x] My change requires a change to the dependencies
  • [x] I have updated the dependencies accordingly
  • [x] I have read the CONTRIBUTING document.
  • [ ] All new and existing tests passed.

valeriupredoi avatar Nov 24 '23 14:11 valeriupredoi

absolutely, I knew I'm missing something, cheers, will do it now :+1:

valeriupredoi avatar Nov 27 '23 13:11 valeriupredoi

Look relay good, probably just needs a poetry lock to update the content-hashin the poetry.lock file :-)

sbrunner avatar Nov 27 '23 14:11 sbrunner

many thanks @sbrunner :beer: The package builds very well too, see the latest build with the more relaxed pins (and build done with Python 3.12) https://github.com/conda-forge/prospector-feedstock/pull/44 Great suggestions!

valeriupredoi avatar Nov 27 '23 14:11 valeriupredoi

as Pierre suggested, I have now removed (yet again) support for python=3.7 (and feeling good about it haha). There is just one test that needs attention, unfortunately I don't know enough prospector to fix it myself (safely/correctly), sorry :beer:

valeriupredoi avatar Dec 04 '23 15:12 valeriupredoi

Not sure about the reason for the pylama bug myself. I also see a bunch of pylint deprecation warnings that do not fair well for pylint 3.0 compat.

Pierre-Sassoulas avatar Dec 04 '23 21:12 Pierre-Sassoulas

happy new year, folks! Just got myself back in work, and was wondering what's the situation of this - should I update pycodestyle too, or just wait for you good folks to get compat with pylint >=3.0 that may have to get in most all these pins anyway? Cheers :beer:

valeriupredoi avatar Jan 12 '24 16:01 valeriupredoi

Hey happy new year @valeriupredoi ! Don't worry about pylint compatibility, let's do thing one by one, if the pipeline is fixed here we're going to merge :)

Pierre-Sassoulas avatar Jan 12 '24 22:01 Pierre-Sassoulas

@Pierre-Sassoulas good man! Looking back I see the first instance of that test fail here https://github.com/landscapeio/prospector/actions/runs/6821193419/job/18554269679 - since all I am doing here is updating pins (hence packages) I am fairly sure it's not something that this PR introduces to make that test fail; comparing environments between my PR and 643, I see quite a lot of dependency version differences, but comparing 643 to the last time the tests passed, ie https://github.com/landscapeio/prospector/actions/runs/6557025877/job/17807920762 I see no difference in terms of deps versions, so it must be a code change that happened sometime before my PR, or, more plausibly, tests fail for forked PRs - could you maybe trigger the tests on the master branch pls see if they still fail? :beer:

valeriupredoi avatar Jan 15 '24 14:01 valeriupredoi

friendly ping here @Pierre-Sassoulas @carlio - apols for being pesky, gents :beer: x2

valeriupredoi avatar Feb 05 '24 13:02 valeriupredoi

Hey I think you're right main branch is broken see dependabot's MR for example: https://github.com/landscapeio/prospector/pull/650

Pierre-Sassoulas avatar Feb 05 '24 14:02 Pierre-Sassoulas

hi folks, what we gonna do about this and support for Python 3.12? Have a good Easter break! :rabbit:

valeriupredoi avatar Mar 26 '24 15:03 valeriupredoi

Hey @valeriupredoi if someone want to open a pull request to fix main I'd review and merge it.

Pierre-Sassoulas avatar Mar 28 '24 21:03 Pierre-Sassoulas

Hey @valeriupredoi if someone want to open a pull request to fix main I'd review and merge it.

cheers, mate! I'd do it meself, but unfortunately my fix would be purely mechanical (just fixing that failing test), I'm afraid I don't have the Prospector-expertise to find the actual root cause :disappointed:

valeriupredoi avatar Apr 03 '24 15:04 valeriupredoi

Hey @valeriupredoi if someone want to open a pull request to fix main I'd review and merge it.

I had a stab at it in https://github.com/landscapeio/prospector/pull/658 - it's not a full root cause fix, but it's something we may be able to live with :grin:

valeriupredoi avatar Apr 04 '24 16:04 valeriupredoi