[workspace] Remove vestigial jupyter package#21085
Conversation
|
@drake-jenkins-bot mac-arm-ventura-unprovisioned-clang-bazel-experimental-release please. To make sure... |
jwnimmer-tri
added
release notes: none
jwnimmer-tri
left a comment
jwnimmer-tri
left a comment
There was a problem hiding this comment.
+(release notes: none) since this is a test-only change.
+@jwnimmer-tri 
Reviewed 1 of 1 files at r1, all commit messages.
Reviewable status: 1 unresolved discussion
a discussion (no related file):
Don't we need the patch to jupyter_notebook_test.py in this PR, too?
svenevs
left a comment
svenevs
left a comment
There was a problem hiding this comment.
Reviewable status: 1 unresolved discussion
a discussion (no related file):
Previously, jwnimmer-tri (Jeremy Nimmer) wrote…
Don't we need the patch to
jupyter_notebook_test.pyin this PR, too?
TBH I'm not sure... why I wanted to re-run unprovisioned CI. However, my understanding is the jupyter command will still exist...
jwnimmer-tri
left a comment
jwnimmer-tri
left a comment
There was a problem hiding this comment.
Reviewable status:
complete! all discussions resolved, LGTM from assignee jwnimmer-tri(platform)
a discussion (no related file):
Previously, svenevs (Stephen McDowell) wrote…
TBH I'm not sure... why I wanted to re-run unprovisioned CI. However, my understanding is the
jupytercommand will still exist...
Fair enough. I guess we'll see!
- https://jupyter.org/install#jupyter-notebook - https://pypi.org/project/jupyter/ (very old...) - https://pypi.org/project/notebook/ (what we want...) Note that `notebook` is already installed via binary_distribution/requirements.txt. Also update to call the `notebook` python command from the same interpreter.
svenevs
force-pushed
the
fix/remove-vestigial-jupyter-package
branch
from
4604362 to
c9c2223
Compare
|
@drake-jenkins-bot mac-arm-ventura-unprovisioned-clang-bazel-experimental-release please. |
svenevs
left a comment
svenevs
left a comment
There was a problem hiding this comment.
Reviewable status:
a discussion (no related file):
Previously, jwnimmer-tri (Jeremy Nimmer) wrote…
Fair enough. I guess we'll see!
OK, it makes more sense to have both here. Re-pushed and relaunched, having this cherry-picked on the other PR makes it easier for me to build the mac images right now.
svenevs
left a comment
svenevs
left a comment
There was a problem hiding this comment.
Reviewed 1 of 1 files at r2.
Reviewable status:
jwnimmer-tri
left a comment
jwnimmer-tri
left a comment
There was a problem hiding this comment.
Reviewed all commit messages.
Reviewable status: 1 unresolved discussion
tools/jupyter/test/jupyter_notebook_test.py line 10 at r2 (raw file):
"""Ensures that `jupyter notebook` is installed (#12042).""" status = subprocess.call( args=[sys.executable, "-m", "notebook", "--help"])
Actually, looking at this more closely, I think we've defeated the purpose of this test.
See #12043. The purpose of this test is to ensure that after the user runs the setup scripts, that jupyter notebook in the terminal works. We should not be changing this test at all.
jwnimmer-tri
left a comment
jwnimmer-tri
left a comment
There was a problem hiding this comment.
Reviewable status: 1 unresolved discussion
tools/jupyter/test/jupyter_notebook_test.py line 10 at r2 (raw file):
Previously, jwnimmer-tri (Jeremy Nimmer) wrote…
Actually, looking at this more closely, I think we've defeated the purpose of this test.
See #12043. The purpose of this test is to ensure that after the user runs the
setupscripts, thatjupyter notebookin the terminal works. We should not be changing this test at all.
The docs at https://jupyter.org/install#jupyter-notebook still say that jupyter notebook is the right way do to things, so this test should have been fine as-is. If it is not passing with python 3.12 setup scripts, then the setup scripts are still broken.
jwnimmer-tri
left a comment
jwnimmer-tri
left a comment
There was a problem hiding this comment.
Reviewable status: 1 unresolved discussion, LGTM missing from assignee jwnimmer-tri(platform)
|
It ended up being a different problem. |
2 participants
Note that
notebookis already installed via binary_distribution/requirements.txt.Via #21013 (review)
This change is