-
Notifications
You must be signed in to change notification settings - Fork 1.8k
jupyter_ext_dev: pull request for jupyter extension 2.x and 3.x recap… #4167
Conversation
setup.py
Outdated
@@ -58,13 +62,25 @@ | |||
import shutil | |||
import sys | |||
|
|||
import jupyterlab |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please add try-catch for this import. The script should not crash when building machine has not installed jupyter.
And I personally suggest put this and jupyter_lab_version = ...
into a function get_jupyter_lab_version()
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
changed, In this way, the building machine without jupyterlab installed will report error and the package related jupyter lab extension will not built, but continue to build other package
setup_ts.py
Outdated
@@ -22,10 +22,11 @@ | |||
import traceback | |||
from zipfile import ZipFile | |||
|
|||
import jupyterlab |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same as setup.py
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
changed
setup_ts.py
Outdated
if Path('ts/jupyter_extension/dist').exists(): | ||
if jupyter_lab_version == '2': | ||
_symlink('ts/jupyter_extension/build', 'nni_node/jupyter-extension') | ||
_symlink(sys.exec_prefix+'/share/jupyter/lab/extensions', 'nni_node/jupyter-extension/extensions') |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Using +
to glue two paths is not recommended.
|
||
def restore_package(): | ||
if jupyter_lab_version == '2': | ||
package_json = json.load(open('ts/jupyter_extension/.package_default.json')) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Delete .package_default.json
or put it into gitignore.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
done, make sense
e76e72c
to
6f264a2
Compare
setup.py
Outdated
if jupyter_lab_version.split('.')[0] != environ_version.split('.')[0]: | ||
sys.exit(f'ERROR: To build a jupyter lab extension, run "JUPYTER_LAB_VERSION={jupyter_lab_version}", current: {environ_version} ') | ||
elif jupyter_lab_version.split('.')[0] != '3': | ||
sys.exit(f'ERROR: To build a jupyter lab extension, run "JUPYTER_LAB_VERSION={jupyter_lab_version}" first for nondefault version(3.x)') |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Bad indent
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
fixed it
6f264a2
to
1e42cf5
Compare
… #4141
recapped: https://github.com/microsoft/nni/pull/4141/commits