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

"preferred" in "Always start the preferred kernel" is not documented #16340

Open
krassowski opened this issue May 17, 2024 · 3 comments
Open

Comments

@krassowski
Copy link
Member

Description

  • the code name uses "default" but the user-facing strings say "preferred"

  • it is not explained anywhere what "preferred" means:

    "autoStartDefaultKernel": {
    "title": "Automatically Start Preferred Kernel",
    "description": "Whether to automatically start the preferred kernel.",
    "type": "boolean",
    "default": false

    ? {
    label: trans.__('Always start the preferred kernel'),
    caption: trans.__(
    'Remember my choice and always start the preferred kernel'
    ),
    checked: autoStartDefault
    }

  • the preference seems to refer to "default" designation in IKernelSearch.options.preference which is ISessionContext.IKernelPreference

    export function getDefaultKernel(
    options: SessionContext.IKernelSearch
    ): string | null {
    const { specs, preference } = options;
    const { name, language, canStart, autoStartDefault } = preference;
    if (!specs || canStart === false) {
    return null;
    }
    const defaultName = autoStartDefault ? specs.default : null;
    if (!name && !language) {
    return defaultName;
    }
    // Look for an exact match of a spec name.
    for (const specName in specs.kernelspecs) {
    if (specName === name) {
    return name;
    }
    }
    // Bail if there is no language.
    if (!language) {
    return defaultName;
    }
    // Check for a single kernel matching the language.
    const matches: string[] = [];
    for (const specName in specs.kernelspecs) {
    const kernelLanguage = specs.kernelspecs[specName]?.language;
    if (language === kernelLanguage) {
    matches.push(specName);
    }
    }
    if (matches.length === 1) {
    const specName = matches[0];
    console.warn(
    'No exact match found for ' +
    specName +
    ', using kernel ' +
    specName +
    ' that matches ' +
    'language=' +
    language
    );
    return specName;
    }
    // No matches found.
    return defaultName;
    }

  • the kernelPreference seems to be an in-memory variable:

    this._kernelPreference = options.kernelPreference ?? {};

    get kernelPreference(): ISessionContext.IKernelPreference {
    return this._kernelPreference;
    }
    set kernelPreference(value: ISessionContext.IKernelPreference) {
    if (!JSONExt.deepEqual(value as any, this._kernelPreference as any)) {
    const oldValue = this._kernelPreference;
    this._kernelPreference = value;
    this._preferenceChanged.emit({
    name: 'kernelPreference',
    oldValue,
    newValue: JSONExt.deepCopy(value as any)
    });
    }
    }

  • only autoStartDefaultKernel but not the id of the default/preferred kernel gets stored in settings

    const updateSessionSettings = (
    session: ISessionContext,
    changes: IChangedArgs<ISessionContext.IKernelPreference>
    ) => {
    const { newValue, oldValue } = changes;
    const autoStartDefault = newValue.autoStartDefault;
    if (
    typeof autoStartDefault === 'boolean' &&
    autoStartDefault !== oldValue.autoStartDefault
    ) {
    // Ensure we break the cycle
    if (
    autoStartDefault !==
    (settings.get('autoStartDefaultKernel').composite as boolean)
    )
    // Once the settings is changed `updateConfig` will take care
    // of the propagation to existing session context.
    settings
    .set('autoStartDefaultKernel', autoStartDefault)
    .catch(reason => {
    console.error(
    `Failed to set ${settings.id}.autoStartDefaultKernel`
    );
    });
    }
    };
    const sessionContexts = new WeakSet<ISessionContext>();
    const listenToKernelPreference = (panel: NotebookPanel): void => {
    const session = panel.context.sessionContext;
    if (!session.isDisposed && !sessionContexts.has(session)) {
    sessionContexts.add(session);
    session.kernelPreferenceChanged.connect(updateSessionSettings);
    session.disposed.connect(() => {
    session.kernelPreferenceChanged.disconnect(updateSessionSettings);
    });
    }
    };

Expected behavior

  • The label in kernel selection dialog is more useful, explaining what "preference" means; maybe it should not say "preferred kernel" but "selected kernel". Maybe it should not say "Always" but "for the duration of this session" (in my understanding the setting is persisted forever, but the "preference" is not)
  • The setting description should explain the behaviour in more detail

Context

  • JupyterLab version: 4.2.0
@krassowski krassowski added bug documentation status:Needs Triage Applied to new issues that need triage labels May 17, 2024
@ericdatakelly
Copy link

"selected kernel" makes a lot of sense. And clarifying that it's for the duration of the session would be very helpful.

@Gecervantes01
Copy link

I'd like to take on this issue!

@RRosio
Copy link
Contributor

RRosio commented May 24, 2024

@Gecervantes01 you are welcome to open a PR for this! 🙂

Gecervantes01 added a commit to DGR-Jupyterlab/jupyterlab that referenced this issue May 26, 2024
…g from "preferred" to "selected", and from "always" to "from the duration of this session". Chages can be seen when clicking on the Kernel tab, then selecting "Change Kernel...".
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

5 participants