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

Update to jupyter-packaging v0.10 #145

Merged
merged 12 commits into from
Sep 21, 2021
Merged

Update to jupyter-packaging v0.10 #145

merged 12 commits into from
Sep 21, 2021

Conversation

hbcarlos
Copy link
Member

@jtpio Can we simplify the development build?

voila-gridstack/setup.py

Lines 69 to 97 in 99c1b35

class DevelopCmd(base_develop_cmd):
prefix_targets = [
("nbconvert/templates", 'gridstack'),
("voila/templates", 'gridstack')
]
def run(self):
target_dir = os.path.join(sys.prefix, 'share', 'jupyter')
if '--user' in sys.prefix: # TODO: is there a better way to find out?
target_dir = jupyter_core_paths.user_dir()
target_dir = os.path.join(target_dir)
for prefix_target, name in self.prefix_targets:
source = os.path.join('share', 'jupyter', prefix_target, name)
target = os.path.join(target_dir, prefix_target, name)
target_subdir = os.path.dirname(target)
if not os.path.exists(target_subdir):
os.makedirs(target_subdir)
rel_source = os.path.relpath(os.path.abspath(source), os.path.abspath(target_subdir))
try:
os.remove(target)
except:
pass
print(rel_source, '->', target)
os.symlink(rel_source, target)
super(DevelopCmd, self).run()
cmdclass['develop'] = DevelopCmd if jupyter_core_paths else base_develop_cmd

@jtpio
Copy link
Member

jtpio commented Sep 16, 2021

Yes I think so.

There used to be a similar one in voila before, which is now gone: https://github.com/voila-dashboards/voila/blob/master/setup.py

@jtpio jtpio mentioned this pull request Sep 16, 2021
5 tasks
@@ -1,19 +1,23 @@
include LICENSE
include README.md
include CHANGELOG.md
Copy link
Member

Choose a reason for hiding this comment

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

We can even combine this line with the one above with *.md.

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes, but I wanted to add both separately to know exactly what we need.
Is the RELEASE.md necessary?

Copy link
Member

Choose a reason for hiding this comment

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

Ah right. Then I guess it's fine to leave it like it is now.

@hbcarlos
Copy link
Member Author

It's done, but I would like to create a symlink to the nbconvert folder for gridstack, when running pip install -e .. That's what this code was doing:

voila-gridstack/setup.py

Lines 69 to 97 in 99c1b35

class DevelopCmd(base_develop_cmd):
prefix_targets = [
("nbconvert/templates", 'gridstack'),
("voila/templates", 'gridstack')
]
def run(self):
target_dir = os.path.join(sys.prefix, 'share', 'jupyter')
if '--user' in sys.prefix: # TODO: is there a better way to find out?
target_dir = jupyter_core_paths.user_dir()
target_dir = os.path.join(target_dir)
for prefix_target, name in self.prefix_targets:
source = os.path.join('share', 'jupyter', prefix_target, name)
target = os.path.join(target_dir, prefix_target, name)
target_subdir = os.path.dirname(target)
if not os.path.exists(target_subdir):
os.makedirs(target_subdir)
rel_source = os.path.relpath(os.path.abspath(source), os.path.abspath(target_subdir))
try:
os.remove(target)
except:
pass
print(rel_source, '->', target)
os.symlink(rel_source, target)
super(DevelopCmd, self).run()
cmdclass['develop'] = DevelopCmd if jupyter_core_paths else base_develop_cmd

Could we create and script or something and run it with npm_builder() in the post-build process?

setup.py Outdated
version=pkg_json["version"],
url=pkg_json["homepage"],
author=pkg_json["author"],
description=pkg_json["description"],
Copy link
Member

Choose a reason for hiding this comment

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

Maybe we still want to keep the metadata for the Python package separate from the JupyterLab extension?

So we don't use this description for the template:

"description": "A JupyterLab extension to create Voila dashboards using GridStack",

Copy link
Member Author

Choose a reason for hiding this comment

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

That's right, I'll change. I'll use the setup.cfg to add the metadata for the python package then.

Copy link
Member

Choose a reason for hiding this comment

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

And we can even move most of this to setup.cfg, similar to: https://github.com/voila-dashboards/voila/blob/main/setup.cfg

@hbcarlos

This comment has been minimized.

@hbcarlos
Copy link
Member Author

I'm setting the version in the setup.cfg manually because that runs the python package during the build process, and it fails because the labextension is not built yet.

@jtpio
Copy link
Member

jtpio commented Sep 17, 2021

I'm setting the version in the setup.cfg manually because that runs the python package during the build process, and it fails because the labextension is not built yet.

Sounds good 👍 We'll want to version the Python package and the lab extension separately anyway.

Later when we do the releaser PR, we can move the Python version to _version.py (and expose it as __version__), and use something like bump2version or tbump to update it (that will be for the version-cmd for the releaser)

pyproject.toml Outdated
@@ -1,3 +1,17 @@
[build-system]
requires = ["jupyter_packaging~=0.7.9", "jupyterlab~=3.0", "setuptools>=40.8.0", "wheel", "jupyter_core"]
build-backend = "setuptools.build_meta"
requires = ["jupyter_packaging~=0.10,<2", "jupyterlab~=3.1", "jupyterlab_widgets~=1.0", "voila>=0.2.0,<0.3.0"]
Copy link
Member

Choose a reason for hiding this comment

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

This could also be:

Suggested change
requires = ["jupyter_packaging~=0.10,<2", "jupyterlab~=3.1", "jupyterlab_widgets~=1.0", "voila>=0.2.0,<0.3.0"]
requires = ["jupyter_packaging~=0.10,<2", "jupyterlab~=3.1", "jupyterlab_widgets~=1.0", "voila~=0.2.0"]

setup.py Outdated Show resolved Hide resolved
Remove useless line

Co-authored-by: Jeremy Tuloup <jeremy.tuloup@gmail.com>
setup.cfg Outdated Show resolved Hide resolved
Copy link
Member

@jtpio jtpio left a comment

Choose a reason for hiding this comment

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

Thanks!

@jtpio
Copy link
Member

jtpio commented Sep 21, 2021

This should be good to merge, and we can iterate afterwards as part of #147

@jtpio jtpio merged commit 33a3f97 into master Sep 21, 2021
@jtpio jtpio deleted the jupyter-packaging branch September 21, 2021 12:04
@hbcarlos
Copy link
Member Author

Thank you so much Jeremy!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants