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

BLD: Use modern jupyter_packaging and jupyterlab #111

Open
wants to merge 8 commits into
base: main
Choose a base branch
from

Conversation

pllim
Copy link
Contributor

@pllim pllim commented Nov 4, 2024

This is so bqplot-image-gl can build on Python 3.13

Also see similar patch at bqplot/bqplot#1653

xref spacetelescope/jdaviz#3210

cc @maartenbreddels @astrofrog

@pllim
Copy link
Contributor Author

pllim commented Nov 4, 2024

Okay so maybe you need to fix this up for jupyterlab 4? That is beyond my skillset here...

@dhomeier
Copy link
Contributor

dhomeier commented Nov 4, 2024

Looks like

"@jupyterlab/builder": "^3.0.0-rc.4",

still pins the npm install to 3.x. No idea if it can safely bumped to ^4.0.0

as suggested by dhomeier
Copy link

codecov bot commented Nov 4, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 44.44%. Comparing base (7b90b9d) to head (c144733).
Report is 8 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main     #111      +/-   ##
==========================================
- Coverage   50.00%   44.44%   -5.56%     
==========================================
  Files           7        8       +1     
  Lines         140      189      +49     
==========================================
+ Hits           70       84      +14     
- Misses         70      105      +35     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@pllim
Copy link
Contributor Author

pllim commented Nov 4, 2024

Oh, thanks, @dhomeier ! CI is green now. :D

@maartenbreddels
Copy link
Collaborator

This looks overal good. Given that we run the visual tests, it seems we can build against jupyter lab 4. Although, I would rather not see us drop 36-38 without a good reason, can we put those back in?

@pllim
Copy link
Contributor Author

pllim commented Nov 12, 2024

Python 3.6-3.8 have reached EOL. You cannot get them on Actions anymore.

@pllim
Copy link
Contributor Author

pllim commented Nov 12, 2024

And even if you can, you will be stuck with a very old stack with no security patches.

@maartenbreddels
Copy link
Collaborator

We still run it every day without a problem: https://github.com/widgetti/solara/actions/runs/11791722711/job/32852280380

I’m a strong advocate against proactively dropping support for older Python versions. In many cases, EOL alone isn’t a sufficient reason to discontinue support, especially in regular user environments without public HTTP servers, where security risks are minimal. Of course, if maintaining compatibility becomes a burden, that’s a valid reason to consider dropping it. But so far, almost all CI I manage runs smoothly on Python 3.6 and above. Admittedly, 3.6 is old, but you’d be surprised how frequently these environments are still in use!”

In any case, apart from this discussion, this PR is about adding 3.13 support, not removing older versions. If you feel very strongly about removing support of older versions, I think it should go into a different PR.

@pllim
Copy link
Contributor Author

pllim commented Nov 12, 2024

Okay I added them back but fixing them is not in my plans.

Copy link
Contributor

@dhomeier dhomeier left a comment

Choose a reason for hiding this comment

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

This may still be running up to a decision between supporting Python 3.13 and 3.6-3.7. Could consider conditional requires, but I'd like to check if the version requirements really need to be that strict – moving to 0.12/4.x did not require any actual code changes, right? So I suggest to try this as a (final?) attempt to keep the older versions.

I have also tried to probe into the failures with py37-macos12, and could not reproduce the installation problems on macOS 10.14, but was unable to build bqplot-image-gl either since pip did not find the npm installation. Perhaps move the py37 tests to a linux host if nothing else helps?

Comment on lines +2 to +3
requires = ["jupyter_packaging~=0.12.2",
"jupyterlab==4.*",
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
requires = ["jupyter_packaging~=0.12.2",
"jupyterlab==4.*",
requires = ["jupyter_packaging>=0.10.6",
"jupyterlab>=3.6",

Those changes are doing away with support for py<3.8, since 3.6 is not supported by jupyter_packaging 0.11+, and jupyterlab 4.0 has removed 3.7.

@pllim
Copy link
Contributor Author

pllim commented Nov 13, 2024

If you insist on supporting very old Python, can you please take over this PR? This is getting very complicated and out of scope of my work to just get this package to build for Python 3.13. Thanks.

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