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

dask_labextension v5.0.2, remove dependency on nodejs, more tests and maintainer #22

Merged
merged 7 commits into from
Jun 3, 2021

Conversation

bollwyvl
Copy link
Contributor

@bollwyvl bollwyvl commented Jun 2, 2021

Checklist

  • Used a personal fork of the feedstock to propose changes
  • Bumped the build number (if the version is unchanged)
  • Reset the build number to 0 (if the version changed)
  • Re-rendered with the latest conda-smithy (Use the phrase @conda-forge-admin, please rerender in a comment in this PR for automated rerendering)
  • Ensured the license file is being packaged.

Thanks for maintaining this, folks!

This (sorta) simplifies the recipe:

  • removing the nodejs dependency altogether
  • deletes pyproject.toml
  • uses the --skip-npm flag

Then makes it more complicated:

  • checks to ensure the extension is installed
  • checks to ensure the plugin.json gets gets deployed (my original issue)
Uncaught (in promise) Error: Schema not found: /zzz/share/jupyter/lab/schemas/dask-labextension/plugin.json
    at Function.create (jlab_core.4cb3c625b5e8bf236b99.js?v=4cb3c625b5e8bf236b99:2)
    at async a.fetch (jlab_core.4cb3c625b5e8bf236b99.js?v=4cb3c625b5e8bf236b99:2)

@conda-forge-linter
Copy link

Hi! This is the friendly automated conda-forge-linting service.

I just wanted to let you know that I linted all conda-recipes in your PR (recipe) and found it was in an excellent condition.

@bollwyvl
Copy link
Contributor Author

bollwyvl commented Jun 2, 2021

@conda-forge-admin please rerender

@bollwyvl
Copy link
Contributor Author

bollwyvl commented Jun 3, 2021

@conda-forge/dask_labextension ready for review!

@bollwyvl bollwyvl changed the title Remove dependency on nodejs, more tests dask_labextension v5.0.2, remove dependency on nodejs, more tests Jun 3, 2021
@bollwyvl
Copy link
Contributor Author

bollwyvl commented Jun 3, 2021

@conda-forge-admin please rerender

@bollwyvl bollwyvl mentioned this pull request Jun 3, 2021
3 tasks
@github-actions
Copy link
Contributor

github-actions bot commented Jun 3, 2021

Hi! This is the friendly automated conda-forge-webservice.
I tried to rerender for you, but it looks like there was nothing to do.

@jsignell
Copy link
Member

jsignell commented Jun 3, 2021

@ian-r-rose @jacobtomlinson you probably have more insight. I can also add you to the maintainer list if you like.

@bollwyvl
Copy link
Contributor Author

bollwyvl commented Jun 3, 2021

add you to the maintainer list

Happy to help!

@bollwyvl
Copy link
Contributor Author

bollwyvl commented Jun 3, 2021

@conda-forge-admin please rerender

@bollwyvl bollwyvl changed the title dask_labextension v5.0.2, remove dependency on nodejs, more tests dask_labextension v5.0.2, remove dependency on nodejs, more tests and maintainer Jun 3, 2021
@ian-r-rose
Copy link

Thanks for this @bollwyvl!

* removing the `nodejs` dependency altogether

👍

* deletes `pyproject.toml`
* uses the `--skip-npm` flag

I admit I'm still a little puzzled as to what best practices are between PEP 517/518 packaging and conda-forge packaging (especially for labextensions and jupyter-packaging). Can you elaborate a bit on these?

* checks to ensure the `plugin.json` gets gets deployed (my original issue)

Thanks for this. Fixes dask/dask-labextension#191

@bollwyvl
Copy link
Contributor Author

bollwyvl commented Jun 3, 2021

puzzled as to what best practices

Yeah, right? Honestly: I don't really know.

It's a non-secret that i'm not a big fan of jupyter-packaging (or really jupyter-* that isn't directly related to the interactive computing experience), and my grumbling has at least gotten it out of a number of run dependencies, as that was already starting to cause problems.

But wait, it's not just jupyter-packaging! This is a problem with poetry-based packages as well.... even though PIP_NO_BUILD_ISOLATION=1 is set, it still sometimes seems to escape, and ends up shipping rando junk in the package. flit seems to Just Work, but then, it doesn't try to reinvent the.... wheel (groan), just implement the spec as closely as possible and offer a backwards-compatible setup.py in sdists. But flit probably won't support data_files unless the situation gets much better, and our explorations of alternatives to the mechanism have been... disheartening jupyter-server/jupyter_server#351

Probably at the end of the day, conda-build will have to start patching pyproject.toml, but until then, gotta keep shipping!

@ian-r-rose
Copy link

I'm happy with this if CI is happy, but if @ocefpaf has any thoughts around pyproject.toml, I'd be curious to hear them

@ocefpaf ocefpaf merged commit 38354e8 into conda-forge:master Jun 3, 2021
@ocefpaf
Copy link
Member

ocefpaf commented Jun 3, 2021

I'm happy with this if CI is happy, but if @ocefpaf has any thoughts around pyproject.toml, I'd be curious to hear them

Sometimes it works as-is, sometimes the build dependencies listed there force us to delete it and let conda take care of the build. I agree with @bollwyvl, this is something that should be fixed in conda-build. Until then, we need the workaround in some places.

@ian-r-rose
Copy link

Thanks @ocefpaf

@bollwyvl
Copy link
Contributor Author

bollwyvl commented Jun 3, 2021 via email

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

Successfully merging this pull request may close these issues.

5 participants