Skip to content

Add support for IPyWidgets 8 #12754

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 22 commits into from
Feb 9, 2023
Merged

Add support for IPyWidgets 8 #12754

merged 22 commits into from
Feb 9, 2023

Conversation

DonJayamanne
Copy link
Contributor

@DonJayamanne DonJayamanne commented Feb 7, 2023 •
edited
Loading

Fixes #8552

@DonJayamanne DonJayamanne changed the title Ipywidgets8 support Add support for IPyWidgets 8 Feb 7, 2023
@DonJayamanne DonJayamanne force-pushed the ipywidgets8Support branch 6 times, most recently from 094de07 to 6bbc470 Compare February 8, 2023 21:50
DonJayamanne
DonJayamanne commented Feb 8, 2023
View reviewed changes
src/kernels/kernelWorkingFolder.node.ts
} else if (kernel.resourceUri && (await this.fs.exists(kernel.resourceUri))) {
} else if (
kernel.resourceUri &&
kernel.resourceUri.scheme !== 'untitled' &&
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Debt = we weren't checking if it was untitled,
No point chekcing if untitled files exist on disc or not, as the answer is obvious

DonJayamanne
DonJayamanne commented Feb 8, 2023
View reviewed changes
src/kernels/kernelProvider.base.ts Outdated
@@ -137,6 +138,7 @@ export abstract class BaseCoreKernelProvider implements IKernelProvider {
`Disposing kernel associated with ${getDisplayPath(notebook.uri)}, isClosed=${notebook.isClosed}`
);
this.pendingDisposables.add(kernelToDispose.kernel);
this.executions.get(kernelToDispose.kernel)?.dispose();
Copy link
Contributor Author

Choose a reason for hiding this comment

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

debt

DonJayamanne
DonJayamanne commented Feb 8, 2023
View reviewed changes
src/kernels/kernelExecution.ts Outdated
@@ -71,6 +72,9 @@ export class NotebookKernelExecution implements INotebookKernelExecution {
kernel.addHook('willRestart', (sessionPromise) => this.onWillRestart(sessionPromise), this, this.disposables);
this.disposables.push(this._onPreExecute);
}
public dispose() {
disposeAllDisposables(this.disposables);
Copy link
Contributor Author

Choose a reason for hiding this comment

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

debt

DonJayamanne
DonJayamanne commented Feb 8, 2023
View reviewed changes
src/kernels/types.ts Outdated
@@ -391,7 +393,7 @@ export interface IKernel extends IBaseKernel {
readonly creator: 'jupyterExtension';
}

export interface INotebookKernelExecution {
export interface INotebookKernelExecution extends IDisposable {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

debt

DonJayamanne
DonJayamanne commented Feb 8, 2023
View reviewed changes
src/notebooks/controllers/vscodeNotebookController.ts
}
scripts.push(
...[
Uri.joinPath(
Copy link
Contributor Author

Choose a reason for hiding this comment

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

All of these are now statically defined in package.json

DonJayamanne
DonJayamanne commented Feb 9, 2023
View reviewed changes
.github/workflows/build-test.yml
pythonVersion: '3.10'
tags: '@widgets'
os: ubuntu-latest
ipywidgetsVersion: '8'
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Added Ci tgests for IPyWidgets 8
We need to have tests for both 7 & 8

DonJayamanne
DonJayamanne commented Feb 9, 2023
View reviewed changes
build/venv-test-ipywidgets8-requirements.txt
livelossplot
versioneer
pythreejs
ipywidgets
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is a copy of venv-test-requirements.txt with the difference of having ipywidgets not pinned to a specific version

@ArthurAsenheimer ArthurAsenheimer mentioned this pull request Feb 9, 2023
Closed
4 tasks
@DonJayamanne DonJayamanne marked this pull request as ready for review February 9, 2023 20:50
@DonJayamanne DonJayamanne enabled auto-merge (squash) February 9, 2023 20:50
amunger
amunger reviewed Feb 9, 2023
View reviewed changes
amunger
amunger reviewed Feb 9, 2023
View reviewed changes
@DonJayamanne DonJayamanne merged commit 790db2a into main Feb 9, 2023
@DonJayamanne DonJayamanne deleted the ipywidgets8Support branch February 9, 2023 22:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@amunger amunger amunger approved these changes

Assignees

@DonJayamanne DonJayamanne

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Add support for ipywidgets 8
2 participants
@DonJayamanne @amunger