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

Handle multiple Python packages #176

Merged
merged 1 commit into from
Oct 13, 2021

Conversation

davidbrochart
Copy link
Contributor

This is needed e.g. for jupyverse, where each plugin is a Python package.
I just want to open a PR early in order to get feedback. This first commit creates a repo with multiple Python packages in it.

Closes #169

@welcome
Copy link

welcome bot commented Oct 6, 2021

Thanks for submitting your first pull request! You are awesome! 🤗

If you haven't done so already, check out Jupyter's Code of Conduct. Also, please make sure you followed the pull request template, as this will help us review your contribution more quickly.
welcome
You can meet the other Jovyans by joining our Discourse forum. There is also a intro thread there where you can stop by and say Hi! 👋

Welcome to the Jupyter community! 🎉

@codecov-commenter
Copy link

codecov-commenter commented Oct 6, 2021

Codecov Report

Merging #176 (4a8290d) into master (e8c7288) will increase coverage by 0.34%.
The diff coverage is 94.87%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #176      +/-   ##
==========================================
+ Coverage   85.51%   85.86%   +0.34%     
==========================================
  Files          17       17              
  Lines        2161     2242      +81     
  Branches      262      276      +14     
==========================================
+ Hits         1848     1925      +77     
- Misses        220      222       +2     
- Partials       93       95       +2     
Impacted Files Coverage Δ
jupyter_releaser/util.py 70.44% <0.00%> (-0.35%) ⬇️
jupyter_releaser/python.py 83.58% <37.50%> (-3.92%) ⬇️
jupyter_releaser/cli.py 91.27% <100.00%> (+0.46%) ⬆️
jupyter_releaser/lib.py 84.19% <100.00%> (ø)
jupyter_releaser/tee.py 82.92% <100.00%> (+0.87%) ⬆️
jupyter_releaser/tests/conftest.py 98.54% <100.00%> (+0.03%) ⬆️
jupyter_releaser/tests/test_cli.py 99.01% <100.00%> (+0.05%) ⬆️
jupyter_releaser/tests/test_functions.py 98.42% <100.00%> (+0.12%) ⬆️
jupyter_releaser/tests/util.py 99.21% <100.00%> (+0.15%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update e8c7288...4a8290d. Read the comment docs.

@jtpio jtpio added the enhancement New feature or request label Oct 7, 2021
@jtpio
Copy link
Member

jtpio commented Oct 7, 2021

Nice, thanks @davidbrochart for looking into this!

@davidbrochart
Copy link
Contributor Author

I'm wondering:

  • should the supported package structure only be of the form: one "main package" at the top-level, and sub-packages in sub-directories?
  • how to handle the version bump of sub-packages? Assuming the package structure is as described above, we currently manually specify the version of the main package, but if sub-packages are released only if they have been changed, manually specifying their new version doesn't make sense. Then, it is basically impossible to follow SemVer, as e.g. only the PATCH version would be incremented, even if there were breaking changes.

@blink1073
Copy link
Contributor

  • We could add config similar to workspaces in package.json as a releaser-specific config.
  • I think we either have to have a single version, or adopt an approach like lumino, where you manually bump versions of the sub-packages and then cut the automated release afterward.

@davidbrochart
Copy link
Contributor Author

  • We could add config similar to workspaces in package.json as a releaser-specific config.

Sounds good, what about a pypackage.json file, with a similar workspaces key?

  • I think we either have to have a single version

Maybe we should go with this, as it's the simplest. Also this is how it's done in JupyterLab, right?

@jtpio
Copy link
Member

jtpio commented Oct 7, 2021

Sounds good, what about a pypackage.json file, with a similar workspaces key?

Or as a releaser option directly? For example:

[tools.jupyter-releaser.options]
python_packages = ["plugins/**", "example_package"]

@blink1073
Copy link
Contributor

Or as a releaser option directly?

Yes, that's what I meant

@davidbrochart
Copy link
Contributor Author

Not sure what's happening on Windows 3.9, since it works on all other platforms.

@davidbrochart
Copy link
Contributor Author

I've been able to release jupyverse and all its plugins with this PR.

Copy link
Contributor

@blink1073 blink1073 left a comment

Choose a reason for hiding this comment

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

Great!

@davidbrochart
Copy link
Contributor Author

hmmm I'm out of ideas. I think I would need a Windows machine with python 3.9 and play with it myself.

@davidbrochart
Copy link
Contributor Author

I'm seeing that in the the logs:

in _run_win C:\Users\runneradmin\AppData\Local\Temp\pytest-of-runneradmin\pytest-0\test_extract_dist_multipy0\.jupyter_releaser_checkout\sub_package1 ['C:/hostedtoolcache/windows/Python/3.9.7/x64/python.EXE', '-m', 'build', '.', '-o', 'C:UsersrunneradminAppDataLocalTemppytest-of-runneradminpytest-0test_extract_dist_multipy0.jupyter_releaser_checkoutdist'] {'shell': True}

The output directory has all / removed 👀

@blink1073
Copy link
Contributor

I think you need to replace os.sep with / in dist_dir

@davidbrochart
Copy link
Contributor Author

So now, replacing os.sep with / on Linux fails, while it should have no effect.

@davidbrochart
Copy link
Contributor Author

My bad, this was it @blink1073, thanks a lot!
Let me squash and remove references to my branch.

Copy link
Contributor

@blink1073 blink1073 left a comment

Choose a reason for hiding this comment

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

Nice work, thank you!

@blink1073 blink1073 merged commit c16943d into jupyter-server:master Oct 13, 2021
@davidbrochart davidbrochart deleted the py_multi_package branch October 13, 2021 16:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Handle multiple Python packages
4 participants