Movatterモバイル変換


[0]ホーム

URL:


Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Sign up
Appearance settings

Use hatch backend#6425

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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.

Already on GitHub?Sign in to your account

Merged
blink1073 merged 10 commits intojupyter:mainfromblink1073:use-hatch
Jun 13, 2022
Merged

Use hatch backend#6425

blink1073 merged 10 commits intojupyter:mainfromblink1073:use-hatch
Jun 13, 2022

Conversation

@blink1073
Copy link
Contributor

@blink1073blink1073 commentedMay 14, 2022
edited
Loading

  • Uses newhatch_jupyter_builder plugin
  • hatch is a newly minted pypa project, and appears to be a drop-in replacement for our use ofsetuptools /jupyter-packaging

jtpio reacted with thumbs up emoji
@github-actions
Copy link
Contributor

Binder 👈 Launch a Binder on branchblink1073/notebook/use-hatch

@blink1073
Copy link
ContributorAuthor

We're blocked bypypa/hatch#243.

@blink1073
Copy link
ContributorAuthor

Found a workaround usingforce-include

@ofek
Copy link

FYI just releasedhttps://github.com/pypa/hatch/releases/tag/hatchling-v1.0.0

Also I think you can copy what you did hereipython/ipyparallel@e3e896a

@blink1073
Copy link
ContributorAuthor

Thanks, and congratulations!

ofek reacted with heart emoji

@blink1073
Copy link
ContributorAuthor

We can removesetup.py oncejupyter-server/jupyter_releaser#319 is released, then this should be good to go!

@ofek
Copy link

It's undocumented until I find an auto-generator for non-Click CLIs, but if build deps are met you can dohatchling version

@blink1073
Copy link
ContributorAuthor

if build deps are met you can do hatchling version

Thanks! Perhapshttps://sphinx-argparse.readthedocs.io/en/stable/?

@blink1073
Copy link
ContributorAuthor

Perhapssphinx-argparse.readthedocs.io/en/stable?

Ah, you are usingmkdocs...

@blink1073
Copy link
ContributorAuthor

@jtpio, any idea why the snapshot tests are failing?

@jtpio
Copy link
Member

It looks like the menu and toolbar customization files are missing (defined inschema) for one of the failures:

image

@blink1073
Copy link
ContributorAuthor

It looks like the menu and toolbar customization files are missing (defined in schema) for one of the failures

Thanks!

@blink1073
Copy link
ContributorAuthor

Hmm, after adding the schemas the tests don't appear to start at all.

@jtpio
Copy link
Member

Hmm, after adding the schemas the tests don't appear to start at all.

hmm strange. For the UI tests the package is first installed with the following:

python -m pip install -vv notebook*.whl

Will check locally.

@jtpio
Copy link
Member

Nice, CI is now passing withfcdb15f.

Looks like there are a few conflicts after the merge of#6429.

add flake8 configadd artifactscleanupuse tbump to get current versioncleanup for new dep versionsclean up workflowsswitch back to bumpversionfix verionfixupfixupfixupfixupfix version checkfixupstry version handling againversion cleanupfix typescript errorundo bump2version changesinclude schemas and add build timeoutsfix workflow syntaxmore workflow cleanupclean up config
@jtpio
Copy link
Member

jtpio commentedMay 31, 2022
edited
Loading

Ah I thought CI was already uploading the built assets as artifacts.

Maybe we could add the following step to the check release workflow so it's easier to inspect the sdist and wheel?

-name:Upload Distributionsuses:actions/upload-artifact@v2with:name:notebook-jupyter-releaser-dist-${{ github.run_number }}path:.jupyter_releaser_checkout/dist

@blink1073
Copy link
ContributorAuthor

blink1073 commentedMay 31, 2022
edited
Loading

Good call. I also played with my wip comparisonscript and got this output now:

python ../extension-cookiecutter-ts/compare.py ../notebook_orig . sdist

Removed_files:notebook-7.0.0a4/jupyter-config/jupyter_server_config.dnotebook-7.0.0a4/jupyter-config/jupyter_server_config.d/notebook.jsonnotebook-7.0.0a4/notebook/labextension/static/remoteEntry.4fb60df0e7df1388ab11.jsAdded files:notebook-7.0.0a4/notebook/labextension/static/remoteEntry.5ea5fe7dc5a5fbc91231.jsnotebook-7.0.0a4/jupyter_config.jsonnotebook-7.0.0a4/lerna.jsonnotebook-7.0.0a4/pyrightconfig.jsonnotebook-7.0.0a4/.eslintrc.json

@blink1073
Copy link
ContributorAuthor

blink1073 commentedMay 31, 2022
edited
Loading

Here's the latest output:

Successfully built notebook-7.0.0a4.tar.gzRemoved_files:notebook-7.0.0a4/jupyter-config/jupyter_server_config.dnotebook-7.0.0a4/notebook/labextension/static/remoteEntry.4fb60df0e7df1388ab11.jsAdded files:notebook-7.0.0a4/notebook/labextension/static/remoteEntry.5ea5fe7dc5a5fbc91231.js
Successfully built notebook-7.0.0a4-py3-none-any.whlRemoved_files:notebook/labextension/static/remoteEntry.4fb60df0e7df1388ab11.jsnotebook-7.0.0a4.data/data/share/jupyter/labextensions/@jupyter-notebook/lab-extension/static/remoteEntry.4fb60df0e7df1388ab11.jsnotebook-7.0.0a4.dist-info/top_level.txtAdded files:notebook-7.0.0a4.data/data/share/jupyter/labextensions/@jupyter-notebook/lab-extension/static/remoteEntry.5ea5fe7dc5a5fbc91231.jsnotebook/labextension/static/remoteEntry.5ea5fe7dc5a5fbc91231.js

@jtpio
Copy link
Member

Looking good!

@jtpio
Copy link
Member

The Binder for this PR fails to build because of the following error:

image

Which seems to be related to thejupyter labextension develop command expecting to find asetup.py:

https://github.com/jupyterlab/jupyterlab/blob/6099bcb4d32902bd61b82df818cdd2df3fe1685c/jupyterlab/federated_labextensions.py#L427-L438

@jtpio
Copy link
Member

Which seems to be related to thejupyter labextension develop command expecting to find asetup.py:

This is likely to be fixed byjupyterlab/jupyterlab#12606.

@blink1073
Copy link
ContributorAuthor

Yeah I think we need to use asetup.py shim for extensions that are targeting versions ofjupyterlab that require it. Once we have 3.x and 4.x versions that don't require it, the cookiecutter itself can be updated to not include it. For the migration script, we can include it if there was already one there and eventually remove it.

@blink1073
Copy link
ContributorAuthor

Drat, now we have the issue that symlinks can't be created over existing files. Withsetuptools the shared data was not written in editable mode. It feels like we need to fix a few bugs and backport them in JupyterLab before doing any migrations. We shouldn'tneed asetup.py and we shouldn't error out when overwriting symlinks.

@ofek
Copy link

ofek commentedJun 1, 2022

Hatchling resolves symlinks fyi

@blink1073
Copy link
ContributorAuthor

The issue is the JupyterLab build command is trying to create symlinks over the files that are placed into/share and causing an error. This issue is purely of our own making. 😄

ofek reacted with thumbs up emoji

@blink1073
Copy link
ContributorAuthor

Ah, the symlink error was actually from a file in this repository, the binder is working now. I actually think this PR is good to go now.

@blink1073blink1073 marked this pull request as ready for reviewJune 10, 2022 22:18
source ="code"

[tool.hatch.build.targets.wheel.shared-data]
"notebook/labextension" ="share/jupyter/labextensions/@jupyter-notebook/lab-extension"

Choose a reason for hiding this comment

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

@blink1073 I'm not an expert on the filesystem layout for JupyterLab extensions, but should this be

Suggested change
"notebook/labextension" ="share/jupyter/labextensions/@jupyter-notebook/lab-extension"
"notebook/labextension" ="share/jupyter/labextensions/@jupyter-notebook"

Copy link
Member

Choose a reason for hiding this comment

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

Looks like this should be kept since it maps to"notebook/labextension" on the left hand side?

Copy link
ContributorAuthor

Choose a reason for hiding this comment

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

Yes, I agree

Copy link

@agoose77agoose77Jun 13, 2022
edited
Loading

Choose a reason for hiding this comment

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

Hmm, I could be wrong but I thought the output directorycontents were supposed to end up inshare/jupyter/labextensions/@jupyter-notebook/

If you look e.g. at the wheel forjupyterlab-katex, the contents of/jupyterlab_katex-3.3.0.data/data/share/jupyter/labextensions/@jupyterlab/katex-extension/ is

schemasstaticinstall.jsonpackage.json

whereas in notebook's wheel we currently have the following files in/notebook-7.0.0a4.data/data/share/jupyter/labextensions/@jupyter-notebook/:

lab-extension

E.g. this isjupyterlab-myst which works:https://github.com/agoose77/jupyterlab-myst/blob/320b15187ee605b1f948619e9e1f9cfa672eaf00/pyproject.toml#L57

Copy link
ContributorAuthor

Choose a reason for hiding this comment

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

The extension is nested in our case, under@jupyter-notebook. The full extension name is@jupyter-notebook/lab-extension.

Choose a reason for hiding this comment

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

Right, I'm with you. This is where the fact that/ is allowed in NPM package names threw me for a second!

@@ -1,4 +0,0 @@
{
"LabApp": {"collaborative":true,"expose_app_in_browser":true },
"JupyterNotebookApp": {"collaborative":true,"expose_app_in_browser":true }
Copy link
Member

Choose a reason for hiding this comment

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

Probably we want to keep this file, so RTC can be tested on Binder?

Copy link
ContributorAuthor

Choose a reason for hiding this comment

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

Restored

Copy link
Member

@jtpiojtpio left a comment
edited
Loading

Choose a reason for hiding this comment

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

Thanks!!

Looking good on Binder and also locally using the assets built by the releaser.

Left a small comment about the config for testing RTC on Binder, but otherwise looks good!

@blink1073blink1073 merged commit614e478 intojupyter:mainJun 13, 2022
@blink1073blink1073 deleted the use-hatch branchJune 13, 2022 15:19
@jtpio
Copy link
Member

Starting a new pre-release with this change.

@jtpio
Copy link
Member

Ah looks like something is wrong with the Python version:#6448

@github-actionsgithub-actionsbot locked asresolvedand limited conversation to collaboratorsJun 14, 2023
Sign up for freeto subscribe to this conversation on GitHub. Already have an account?Sign in.

Reviewers

@jtpiojtpiojtpio approved these changes

+2 more reviewers

@agoose77agoose77agoose77 left review comments

@ofekofekofek left review comments

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Projects

None yet

Milestone

7.0

Development

Successfully merging this pull request may close these issues.

4 participants

@blink1073@ofek@jtpio@agoose77

[8]ページ先頭

©2009-2025 Movatter.jp