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

General reorganization and modernization of the repository #1140

Merged
merged 18 commits into from
Oct 19, 2023

Conversation

mahendrapaipuri
Copy link
Contributor

Prologue:

This PR is the work started in #1134. The aforementioned PR is closed in favour of the current one to have a cleaner and more scoped discussion.

Description:

This PR attempts to reorganize repo to src-layout, migrate the package to hatch build system which is being used in the Jupyter ecosystem and also address PEP621 by moving all the package metadata to pyproject.toml.

New organization:

Only important folders are listed in the tree

.
├── binder
├── demo
├── docs
├── jupyterlab
│   ├── jupyter-config
│   ├── jupyterlab_jupytext
│   └── packages
│       └── jupyterlab-jupytext-extension
│           ├── src
│           └── style
├── src
│   ├── jupytext
│   └── jupytext_config
└── tests

Breaking changes:

  • The organization of repository has been changed to src-layout to put all core modules of jupytext in src/ folder
  • A separate new module jupytext_config has been created with same behaviour as jupytext-config console script
  • The JupyterLab extension jupyterlab_jupytext has been moved into jupyterlab/ folder including all jupyter related config files. jupyterlab_jupytext will be distributed as another python module along with jupytext, jupytext_config. All the jupyter_server and jupyterlab related extension registration has been moved into jupyterlab_jupytext module.
  • Extension build process can be skipped using HATCH_BUILD_HOOKS_ENABLE=false env var for local dev.

TLDR;
Instead of distrtributing a single jupytext package (which is the current case now), we split it into three jupytext (core), jupytext_config (config) and jupyterlab_jupytext (Jupyter server and lab extension). Building of labextension can be using HATCH_BUILD_HOOKS_ENABLE=false env var when building the package in local dev env.

List of changes:

  • Minimum Python version bumped to 3.8 inline with JupyterLab 4
  • sphinx-gallery dependency has been removed
  • All package metadata has been moved to pyproject.toml
  • Unused tox config has been removed
  • Lab extension's dependencies have been bumped to JupyterLab 4
  • Lint config has been added to TS extension and a TS lint test in CI build step has been as well
  • CI workflow file has been split into multiple step files
  • jupyter-maintainer-tools are used to setup python and node in CI workflows

Todo:

  • Documentation changes to be made once we agree on organizational changes to the repo.

Closes #1076
Closes #1114
Closes #1135

* Move jupytext module inside src in root

* Create a new module jupytext_config with same behaviour as console script
* Not relevant with Nb 7
* Folder jupyterlab in root will contain all jupyter related stuff

* More TS packages must be added in jupyterlab/packages folder

* Added empty style files

* Removed duplicate LICENSE file
* Use top level package.json to build all extensions at once

* Add lint config to lint TS src

* Add yarn config file that is needed for Yarn 3

* Bump  jupyterlab-jupytext extension node deps to JLab 4

* Modify jupyterlab-jupytext extension's package.json for new repo structure
* Jupyter server and lab extension points are moved to jupyterlab module

* Names are modified appropriately in config files
* Use pyproject.toml inline with PEP621

* Remove unused pylint cfg and add ruff tool

* Remove individual cfg files as everything is consolidated in pyproject.toml
* Lint contents of jupyterlab

* Add yarn.lock for reproducible builds
* Split workflows into logical steps and use nested workflow approach

* General maintainence on workflows to work with new build organization and build system

* Install kernel when quatro is installed. Seems like CI fails otherwise
https://github.com/mahendrapaipuri/jupytext/actions/runs/6561210848/job/17820549979

* Add alls-green action to show CI results in more UI friendly way

* Use optional uploading of build artifacts
@LecrisUT
Copy link
Contributor

Thank you for the rebase 😃. Don't know if I can look at it until Friday. If there's something specific, ping me.

@mahendrapaipuri
Copy link
Contributor Author

The original #1134 became a nightmare for me to follow the discussion. And GitHub rate limiting made it even worst with ever loading pages. I cherry picked and reworked on commits to have a cleaner git history (at least I tried my best).

@LecrisUT I understand that you have a strong preference on how workflows should be organization, fair enough. Let's not again open the can of worms here. I would suggest you to work on a separate PR once this one gets merged. It will be more faster and efficient as you have a clear vision on how the CI workflows should be organized. In any case, I tried to bring as many changes in CI workflows as I can to the current one from the discussion in #1134 as we spent already quite a lot of time. Lets try to strictly scope the discussion for important changes and work on the rest in subsequent PRs.

Unfortuanately, index.ts is still being shown as "new" file on Github UI. However, that is not the case with native git show

diff --git a/jupyterlab/packages/jupyterlab-jupytext-extension/src/index.ts b/jupyterlab/packages/jupyterlab-jupytext-extension/src/index.ts
index 5ccc706..06a2c10 100644
--- a/jupyterlab/packages/jupyterlab-jupytext-extension/src/index.ts
+++ b/jupyterlab/packages/jupyterlab-jupytext-extension/src/index.ts
@@ -1,41 +1,39 @@
 import {
   JupyterFrontEnd,
   JupyterFrontEndPlugin,
-} from "@jupyterlab/application";
+} from '@jupyterlab/application';
 
 import {
   ICommandPalette,
-  ISessionContextDialogs,
   showErrorMessage,
   IToolbarWidgetRegistry,
   createToolbarFactory,
-} from "@jupyterlab/apputils";
+} from '@jupyterlab/apputils';
 
 import { ISettingRegistry } from '@jupyterlab/settingregistry';
 
-import { IEditorServices } from "@jupyterlab/codeeditor";
+import { IEditorServices } from '@jupyterlab/codeeditor';
 
-import * as nbformat from "@jupyterlab/nbformat";
+import * as nbformat from '@jupyterlab/nbformat';

Seems like its a bug in Github.

I have also removed all the files and config related to jupyter-releaser as it is creating too much confusion. I have created an issue #1141 so we can track it in a different PR, if maintainers are interested.

@mwouts Cheers for testing build. It was actually a problem with hatch build config. I corrected it and everything should work on this branch.

.github/workflows/ci.yml Outdated Show resolved Hide resolved
@mahendrapaipuri
Copy link
Contributor Author

@github-advanced-security
Copy link

This pull request sets up GitHub code scanning for this repository. Once the scans have completed and the checks have passed, the analysis results for this pull request branch will appear on this overview. Once you merge this pull request, the 'Security' tab will show more code scanning analysis results (for example, for the default branch). Depending on your configuration and choice of analysis tool, future pull requests will be annotated with code scanning analysis results. For more information about GitHub code scanning, check out the documentation.

@mahendrapaipuri mahendrapaipuri changed the title General reorganization modernization of the repository General reorganization and modernization of the repository Oct 18, 2023
Copy link
Owner

@mwouts mwouts left a comment

Choose a reason for hiding this comment

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

Hi @mahendrapaipuri , thank you for this. This is awesome.

I have been able to build and test the package locally and I am very happy with it.

I have left a couple of comments, most of them are questions, only two of them really matter to me: 1. the size of the package (too big, I'd rather not include the tests) and 2. I'd prefer to keep jupytext.TextFileContentsManager for backward compatibility.

I am fine with merging your PR in its current state or with the two points above corrected, as you prefer.

Thank you again for this beautiful piece!

.github/workflows/step_build.yml Show resolved Hide resolved
.github/workflows/step_build.yml Show resolved Hide resolved
.pre-commit-config.yaml Outdated Show resolved Hide resolved
binder/postBuild Outdated Show resolved Hide resolved
jupyterlab/jupyterlab_jupytext/__init__.py Show resolved Hide resolved
@mwouts
Copy link
Owner

mwouts commented Oct 19, 2023

Hi @mahendrapaipuri , FYI I have started working on the changelog/doc update, on top of your PR, cf. https://github.com/mwouts/jupytext/tree/version_1.16-dev

Also, I think I would like to publish a version 1.3.10 of the npm package jupyterlab-jupytext with the current extension content but I've had no time to do this yet, but I guess it should not be too much trouble to rebase your PR on that once I am done.

@mahendrapaipuri
Copy link
Contributor Author

@mwouts No problem, should not be an issue !!

@mahendrapaipuri
Copy link
Contributor Author

@mwouts If you install hatch in your local dev, you can bump versions using hatch version cmd. More in docs

@mwouts
Copy link
Owner

mwouts commented Oct 19, 2023

I'm good with this version, so I'll let the CI finish and merge this PR. Thank you again @mahendrapaipuri !

@codecov
Copy link

codecov bot commented Oct 19, 2023

Codecov Report

Merging #1140 (22b1c78) into main (7a18285) will increase coverage by 0.26%.
The diff coverage is 100.00%.

@@            Coverage Diff             @@
##             main    #1140      +/-   ##
==========================================
+ Coverage   94.73%   94.99%   +0.26%     
==========================================
  Files         123       29      -94     
  Lines       11290     4419    -6871     
==========================================
- Hits        10696     4198    -6498     
+ Misses        594      221     -373     
Files Coverage Δ
src/jupytext/__init__.py 100.00% <100.00%> (ø)
src/jupytext/__main__.py 0.00% <ø> (ø)
src/jupytext/cell_metadata.py 98.99% <ø> (ø)
src/jupytext/cell_reader.py 96.79% <ø> (ø)
src/jupytext/cell_to_text.py 97.83% <ø> (ø)
src/jupytext/cli.py 95.85% <ø> (ø)
src/jupytext/combine.py 100.00% <ø> (ø)
src/jupytext/compare.py 98.75% <ø> (ø)
src/jupytext/config.py 97.50% <ø> (ø)
src/jupytext/contentsmanager.py 94.90% <ø> (ø)
... and 19 more

... and 95 files with indirect coverage changes

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

@mwouts mwouts merged commit a2b9b26 into mwouts:main Oct 19, 2023
21 of 23 checks passed
@mahendrapaipuri mahendrapaipuri mentioned this pull request Oct 19, 2023
13 tasks
@mahendrapaipuri mahendrapaipuri deleted the modernize_build_infra branch November 27, 2023 21:40
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.

sphinx-gallery and rst2md "jupyter labextension list" report issue for JupyterLab 4.0.2 PEP621
3 participants