mwouts / jupytext

Jupyter Notebooks as Markdown Documents, Julia, Python or R scripts
https://jupytext.readthedocs.io
MIT License
6.6k stars 386 forks source link

Keep using jupytext.json for configuring the extensions #1186

Closed mwouts closed 10 months ago

mwouts commented 10 months ago

Closes #1185

github-actions[bot] commented 10 months ago

Thank you for making this pull request.

Did you know? You can try it on Binder: Binder:lab or Binder:notebook.

Also, the version of Jupytext developed in this PR can be installed with pip:

pip install git+https://github.com/mwouts/jupytext.git@rename_jupyterlab_jupytext_to_jupytext

(this requires nodejs, see more at Developing Jupytext)

mwouts commented 10 months ago

@mahendrapaipuri I would prefer to keep the setting files unchanged, to avoid getting in trouble with e.g. older configuration files left behind. Can you have a quick look at this PR and let me know what you think? Thanks!

codecov[bot] commented 10 months ago

Codecov Report

All modified and coverable lines are covered by tests :white_check_mark:

Comparison is base (7223622) 97.73% compared to head (28665a8) 97.73%.

Additional details and impacted files ```diff @@ Coverage Diff @@ ## main #1186 +/- ## ======================================= Coverage 97.73% 97.73% ======================================= Files 29 29 Lines 4456 4456 ======================================= Hits 4355 4355 Misses 101 101 ``` | [Flag](https://app.codecov.io/gh/mwouts/jupytext/pull/1186/flags?src=pr&el=flags&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Marc+Wouts) | Coverage Δ | | |---|---|---| | [external](https://app.codecov.io/gh/mwouts/jupytext/pull/1186/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Marc+Wouts) | `75.17% <ø> (ø)` | | | [functional](https://app.codecov.io/gh/mwouts/jupytext/pull/1186/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Marc+Wouts) | `88.56% <ø> (ø)` | | | [integration](https://app.codecov.io/gh/mwouts/jupytext/pull/1186/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Marc+Wouts) | `77.31% <ø> (ø)` | | | [unit](https://app.codecov.io/gh/mwouts/jupytext/pull/1186/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Marc+Wouts) | `66.56% <ø> (ø)` | | Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Marc+Wouts#carryforward-flags-in-the-pull-request-comment) to find out more.

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

mahendrapaipuri commented 10 months ago

When we reorganized the repository, we made a decision to split core jupytext and jupyterlab extension. My opinion is that we should try to keep it that way for better maintainability.

But the issue you raised in #1185 is a valid one. What we can do is add a build hook that cleans up the previous extension config file. As you mentioned in the issue, the error is non fatal. Jupyter server is simply trying to load a non existent extension. So, this is not a real blocker for the users. Is this sort of solution you are willing to accept? If so, I can try to draft a quick PR.

mahendrapaipuri commented 10 months ago

I guess there is even a simpler solution to this. We can rename jupyterlab-jupytext.json file to jupytext.json. This overwrites the legacy config file (if present in the env) from previous versions and we get rid of these warnings. Could you give it a try?

mwouts commented 10 months ago

Oh yes, that would be a nice way to do it! I will give it a try tomorrow. Thanks!

mwouts commented 10 months ago

Hey @mahendrapaipuri , just to double check, is this new change what you suggested above ? Thanks !

mahendrapaipuri commented 10 months ago

@mwouts Exactly. This will overwrite existing config file (if any) in the current env.