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

a helper tool for initializing defaultViewers in labconfig/ #1094

Merged
merged 10 commits into from
Aug 26, 2023
Merged

a helper tool for initializing defaultViewers in labconfig/ #1094

merged 10 commits into from
Aug 26, 2023

Conversation

parmentelat
Copy link
Contributor

with nb7 about to be released, the default versions of jupyterlab(4)/notebook(7) will both require to configure defaultViewers if one wants, for example, to easily open text notebooks from the web app, or to be able to open notebooks through a URL (these settings were not required with nbclassic)

here's a typical setup (this one is right for my use case, but more languages could be added of course)

cat ~/.jupyter/labconfig/default_setting_overrides.json
{
    "@jupyterlab/docmanager-extension:plugin": {
        "defaultViewers": {
            "python": "Jupytext Notebook",
            "markdown": "Jupytext Notebook"
        }
    }
}

So I've been thinking that providing a simple way to safely implement these settings would be helpful, especially for large classes of beginner students

Here's a POC for that

Of course it would need to be integrated more nicely, and could be exposed as either

  • jupytext init
  • jupytext --init

or any variant, to be discussed...

the first form would resonate with conda init bash but it might have ambiguous meaning (and/or be harder to implement)
what if a file is named init ?
on the other hand, there's a precedent with
jupyter notebook mynotebook and jupyter notebook list that behave differently - so in this instance too, what if your notebook is called list ?

anyways, just throwing the idea in the air, I just feel like having this could save a lot of frustration

@parmentelat
Copy link
Contributor Author

browsing the doc, I come across this section
https://jupytext.readthedocs.io/en/latest/install.html#jupytext-s-contents-manager
that mentions
.jupyter/jupyter_notebook_config.py
which is the nbclassic side of configuring for jupytext

this could be taken care of in jupytext init as well, and would simplify the doc as a side effect

@mwouts
Copy link
Owner

mwouts commented Jul 3, 2023

Hey @parmentelat , thank you for suggesting this.

This reminds me of some config file that are installed by the pip package, and activate the extensions bundled with Jupytext:
https://github.com/mwouts/jupytext/blob/main/setup.py#L82-L92

Obviously, making text files open as notebook with a single click for every Jupytext user would be quite a strong move and might not be appropriate, but maybe there's something interesting in that approach? I.e. do you think the setting could be an extension that the users could activate by typing something like jupyter labextension activate open-text-files-as-notebooks?

@parmentelat
Copy link
Contributor Author

let me clarify
I am not suggesting that this be done at install-time
More like exposing a tool that can be triggered explicitly to perform this configuration:

  • more simply than having to juggle with a 5-lines bash snippet, that in addition won't work on a bare windows install
  • and more safely too (create the folder if missing, add settings if some are already present, etc..)

so I was more hoping that my installation instructions could just read

pip install jupytext
jupytext init

or something along these lines

@parmentelat
Copy link
Contributor Author

@mwouts
let me ping you on this one
I'd have some cycles in the coming weeks, and would ideally like to have this shipped when my students show up in early september; I can take care of mostly everything, but would love to reach a consensus on the proposed approach
so your inputs on a potential way forward would be great :)

@parmentelat
Copy link
Contributor Author

hi @mwouts
ping again

I'm not sure that I fully understand your suggestion about
https://github.com/mwouts/jupytext/blob/main/setup.py#L82-L92

again I was just proposing that, instead of instructing my newcomer students to mess with the file
(i.e. creating the folder(s), open an editor, screw up the json syntax, ...)
instead of that they would be able to implement this settings using a simple jupytext init
much like / inspired by e.g. conda init bash

this would be an explicit choice indeed

I do not know enough about the jupyter labextension subcommand, plus, my main objective really is to keep it as simple as possible, hence my proposal

@parmentelat
Copy link
Contributor Author

parmentelat commented Jul 26, 2023

I went ahead and the latest tip of that branch has support for

  • jupytext --init
    defines the defaultViewer for all 7 languages :
    "python", "markdown", "myst", "r-markdown", "quarto", "julia", "r",

  • jupytext --init markdown python
    same but on a specific subset of langs

if you first use jupytext --init, running it again with a smaller set of languages, it will not remove any setting; generally speaking in this version it never removes a setting

@parmentelat
Copy link
Contributor Author

@mwouts you are loudly silent on this thread, is there any chance I can get you to accept this PR ?

believe me it would really really improve many teachers' life :)

@mwouts
Copy link
Owner

mwouts commented Aug 9, 2023

Thank you @parmentelat for this PR, I agree that it addresses the need of configuring the default viewers in Jupyter Lab/Notebook.

I am sorry I have not answered earlier, this was because 1. your other PR seemed more urgent and 2. I have not taken/had time to test this myself yet, and maybe also 3. until now the configuration was done through jupyter commands, not jupytext, so I do have a little hesitation to add this to the jupytext command (but maybe a) it does make sense or b) we could create another command especially for this, what do you think?)

@parmentelat
Copy link
Contributor Author

Hi @mwouts

in my use case, the feature addresses newbie L3 students, and during the very first course we have them install the software stack that they will use throughout the year to read the course notebooks
so my major objective here is simplicity; I need something easy to remember and to type

I have seen your proposal that was about using instead
jupyter labextension activate open-text-files-as-notebooks
honestly, this imho is not right, it is too cumbersome even for me to remember that...
in addition, in practical terms, it feels like using a jupyter command is going to take a lot of discussions, and thus delay; and ideally I'd need this in a 3-week time when the students turn up !

plus, because we already have miniconda as part of the stack, my first inclination had been to propose jupytext init just like we have also conda init bash

the current code actually implements jupytext --init because it the simplest road to a running code, and plus it allowed to specify a list of languages on the command line, so it looked OK, but this can be changed as per your feedback

now if you'd rather that we define a new command instead, like, I don't know
jupytext-as-default-viewer,
or something to that effect, that would work for me too

so to summarize, anything that can be made available for sept. 4 and that's reasonably easy to remember would work for me :-)

@mwouts
Copy link
Owner

mwouts commented Aug 14, 2023

Hey @parmentelat, no worries, I pretty am sure we will be able to meet the deadline on this one!

What would you think about a new command line tool that we would call like this:

jupytext-config set-default-viewer python markdown

The idea is that later on maybe jupytext-config would be able to show/edit the Jupyter/Jupytext configuration

@parmentelat
Copy link
Contributor Author

Hey @mwouts
That sounds fine to me
How would you like to proceed ? I can update the PR to implement this if you wish

@mwouts
Copy link
Owner

mwouts commented Aug 17, 2023

Hey @parmentelat , thanks for offering this. I think I will need to experiment a bit with this on my end too, so maybe I give it a try this weekend and I make a PR on top of yours ?

@parmentelat
Copy link
Contributor Author

Hi @mwouts
I went ahead and came up with a draft that would do what we have agreed upon
I have added another subcommand while I was at it: jupytext-config list
of course this is just a suggestion and please change or rewrite as you see fit

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.

Thank you @parmentelat for providing this!

This sounds great. I have added a few comments about subcommand names/help if you don't mind to address them.

Also, would you mind updating the documentation under https://github.com/mwouts/jupytext/blob/main/docs/text-notebooks.md#with-a-single-click to mention the new tool jupytext-config? We can probably remove the GIF and the other alternatives (e.g. the wget command).

Also in the section https://github.com/mwouts/jupytext/blob/main/docs/text-notebooks.md#how-to-open-a-text-notebook-in-jupyter-notebook, I think we should mention that from notebook>=7 on, text notebooks open with the editor by default, but that this can be changed with either jupytext-config or with the advanced settings editor.

Kind regards,

jupytext/jupytext_config.py Outdated Show resolved Hide resolved
jupytext/jupytext_config.py Show resolved Hide resolved
jupytext/jupytext_config.py Outdated Show resolved Hide resolved
subparser.set_defaults(subcommand=subcommand)
subcommand.fill_parser(subparser)
args = parser.parse_args()
return args.subcommand.main(args)
Copy link
Owner

Choose a reason for hiding this comment

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

When we run jupytext-config with no argument, the output is like this:

usage: jupytext-config [-h] {list,set-default-viewer} ...
jupytext-config: error: the following arguments are required: {list,set-default-viewer}

If possible I would prefer it to say one of the following sub-commands is required.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

see df4dcec
I'm not exactly comfy enough with subparsers to do what you ask without heavy surgery
so this is not quite what you asked, but without argument we now show the --help message, which is a little more verbose

@parmentelat
Copy link
Contributor Author

hi @mwouts
thanks for reviewing this first draft; you're making a lot of sense
please allow for a few days and I will take all this into account
I'm just curious about the failing actions; I guess the CI/test-pip thing can be safely ignored for both 3.12-dev and 3.13-dev, but what about (3.11, ~=4.0) ? is that something that I need to worry about ?

@parmentelat
Copy link
Contributor Author

I made all the code changes

as far as the doc, I have a question about the With a right click section, that currently suggests to use the Notebook entry in the Open with submenu
but my understanding was that it should rather be the Jupytext Notebook entry instead, no ?

@parmentelat parmentelat deleted the init-settings branch August 21, 2023 20:14
@parmentelat parmentelat restored the init-settings branch August 21, 2023 20:15
@parmentelat parmentelat reopened this Aug 21, 2023
@parmentelat
Copy link
Contributor Author

oops, sorry, I was trying to rename in my repo the branch that this PR is built against, it did not work as I expected...

@parmentelat
Copy link
Contributor Author

so, trying to clarify the behaviour of the 2 entries in the Open With submenu:

both Notebook and Jupytext Notebook behave the same, i.e. each will result in opening a new tab with the contents opened as a notebook

and given that the default viewer with jupytext-config set-default-viewer is Jupytext Notebook, it feels better if the docs to advise to use the Jupytext Notebook viewer

I mean, if I do Open withNotebook and then later I double click on the same notebook, I will get 2 tabs with the same contents...

what do you think @mwouts ?

@mwouts
Copy link
Owner

mwouts commented Aug 23, 2023

Hey @parmentelat , thank you for the changes!

I'm just curious about the failing actions; I guess the CI/test-pip thing can be safely ignored for both 3.12-dev and 3.13-dev, but what about (3.11, ~=4.0) ? is that something that I need to worry about ?

These are three experimental builds, they are allowed to fail. Version 4.0 of markdown-it-py does not exist yet, the build is there to start testing it when it will lands (but, since it is allowed to fail maybe it's not adding much 😕)

I have a question about the With a right click section, that currently suggests to use the Notebook entry in the Open with submenu, but my understanding was that it should rather be the Jupytext Notebook entry instead, no ?

Well historically we only had a Notebook entry. We had to add Jupytext Notebook to the list of editors to allow opening .md links as notebooks if I remember correctly. As you report both behave the same - in the end they both open the document with the notebook editor. If we can continue to use Notebook I have a preference for that as for me the existence of the Jupytext Notebook entry in that menu is just a undesired consequence of allowing to use the notebook editor for text files.

@parmentelat
Copy link
Contributor Author

Hi @mwouts
I'm all done with this PR I believe:

  • I have added a
    jupytext-config unset-default-viewer subcommand for consistency
  • I have lightly rewritten docs/text-notebooks.md as per your suggestions

please review the latter and let me know of anything that needs further trimming

@parmentelat
Copy link
Contributor Author

last minute: I came across another section in the doc that needed a tweak about the classic notebook

@mwouts
Copy link
Owner

mwouts commented Aug 26, 2023

Hey @parmentelat , this looks great! Let me know when you want me to release this.

@parmentelat
Copy link
Contributor Author

Well as far as I am concerned this is good to fly :)
Just please be aware that I have referred to this version as 1.15.1 in the docs:

$ grep 1\.15\.1 docs/*.md
docs/install.md:*Note*: as of this writing (version 1.15.1) this section **applies only to
docs/text-notebooks.md:Since version 1.15.1, `jupytext` comes with a helper command that allows you to

so you may want to tweak these files if you want to issue this under another version number

@mwouts mwouts merged commit 9bedfb7 into mwouts:main Aug 26, 2023
@mwouts
Copy link
Owner

mwouts commented Aug 26, 2023

Awesome ! Yes 1.15.1 is the version number that I am planning to use for the new release.

@parmentelat parmentelat deleted the init-settings branch September 7, 2023 11:38
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.

2 participants