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

Make notebook support optional #52

Closed
wimglenn opened this issue Oct 26, 2023 · 2 comments · Fixed by #54
Closed

Make notebook support optional #52

wimglenn opened this issue Oct 26, 2023 · 2 comments · Fixed by #54
Assignees

Comments

@wimglenn
Copy link

wimglenn commented Oct 26, 2023

For 1.4 the dependency tree of removestar looked like

removestar<=1.4
└── pyflakes

After #31 it looks like

removestar==1.5
├── nbconvert
│   ├── beautifulsoup4
│   │   └── soupsieve>1.2
│   ├── bleach!=5.0.0
│   │   ├── six>=1.9.0
│   │   └── webencodings
│   ├── defusedxml
│   ├── jinja2>=3.0
│   │   └── MarkupSafe>=2.0
│   ├── jupyter-core>=4.7
│   │   ├── platformdirs>=2.5
│   │   └── traitlets>=5.3
│   ├── jupyterlab-pygments
│   ├── markupsafe>=2.0
│   ├── mistune<4,>=2.0.3
│   ├── nbclient>=0.5.0
│   │   ├── jupyter-client>=6.1.12
│   │   │   ├── jupyter-core!=5.0.*,>=4.12
│   │   │   │   ├── platformdirs>=2.5
│   │   │   │   └── traitlets>=5.3
│   │   │   ├── python-dateutil>=2.8.2
│   │   │   │   └── six>=1.5
│   │   │   ├── pyzmq>=23.0
│   │   │   ├── tornado>=6.2
│   │   │   └── traitlets>=5.3
│   │   ├── jupyter-core!=5.0.*,>=4.12
│   │   │   ├── platformdirs>=2.5
│   │   │   └── traitlets>=5.3
│   │   ├── nbformat>=5.1
│   │   │   ├── fastjsonschema
│   │   │   ├── jsonschema>=2.6
│   │   │   │   ├── attrs>=22.2.0
│   │   │   │   ├── jsonschema-specifications>=2023.03.6
│   │   │   │   │   └── referencing>=0.28.0
│   │   │   │   │       ├── attrs>=22.2.0
│   │   │   │   │       └── rpds-py>=0.7.0
│   │   │   │   ├── referencing>=0.28.4
│   │   │   │   │   ├── attrs>=22.2.0
│   │   │   │   │   └── rpds-py>=0.7.0
│   │   │   │   └── rpds-py>=0.7.1
│   │   │   ├── jupyter-core
│   │   │   │   ├── platformdirs>=2.5
│   │   │   │   └── traitlets>=5.3
│   │   │   └── traitlets>=5.1
│   │   └── traitlets>=5.4
│   ├── nbformat>=5.7
│   │   ├── fastjsonschema
│   │   ├── jsonschema>=2.6
│   │   │   ├── attrs>=22.2.0
│   │   │   ├── jsonschema-specifications>=2023.03.6
│   │   │   │   └── referencing>=0.28.0
│   │   │   │       ├── attrs>=22.2.0
│   │   │   │       └── rpds-py>=0.7.0
│   │   │   ├── referencing>=0.28.4
│   │   │   │   ├── attrs>=22.2.0
│   │   │   │   └── rpds-py>=0.7.0
│   │   │   └── rpds-py>=0.7.1
│   │   ├── jupyter-core
│   │   │   ├── platformdirs>=2.5
│   │   │   └── traitlets>=5.3
│   │   └── traitlets>=5.1
│   ├── packaging
│   ├── pandocfilters>=1.4.1
│   ├── pygments>=2.4.1
│   ├── tinycss2
│   │   └── webencodings>=0.4
│   └── traitlets>=5.1
├── nbformat
│   ├── fastjsonschema
│   ├── jsonschema>=2.6
│   │   ├── attrs>=22.2.0
│   │   ├── jsonschema-specifications>=2023.03.6
│   │   │   └── referencing>=0.28.0
│   │   │       ├── attrs>=22.2.0
│   │   │       └── rpds-py>=0.7.0
│   │   ├── referencing>=0.28.4
│   │   │   ├── attrs>=22.2.0
│   │   │   └── rpds-py>=0.7.0
│   │   └── rpds-py>=0.7.1
│   ├── jupyter-core
│   │   ├── platformdirs>=2.5
│   │   └── traitlets>=5.3
│   └── traitlets>=5.1
└── pyflakes

Can the heavy nb dependencies be moved to an optional extra?

I think tools like this should be kept lightweight so we have the option to install them directly in env without creating conflicts. With this tree a pipx-style installation is warranted.

@Saransh-cpp
Copy link
Collaborator

Saransh-cpp commented Oct 26, 2023

Oh yes, that should be done. Thanks for reporting this! General questions (for watchers and the future me) -

  • should removestar throw a warning if nbconvert and nbformat is not installed?
  • should this be a part of v1.6.0 or. v1.5.1?
  • is this a breaking change that needs a new major version? (v2.0.0 - I don't think so)

@wimglenn
Copy link
Author

My recommendation would be to isolate the code which requires nbconvert/nbformat into a separate submodule. That way when a user attempts to use those features without necessary deps in place you can handle the import error and print some message about installing with the [extra]. Or just let the ImportError blow up, that might be self-explanatory enough for the target audience.

@Saransh-cpp Saransh-cpp self-assigned this Oct 27, 2023
@Saransh-cpp Saransh-cpp linked a pull request Nov 1, 2023 that will close this issue
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 a pull request may close this issue.

2 participants