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

Optional dependencies #22

Open
ghost opened this issue May 24, 2021 · 4 comments
Open

Optional dependencies #22

ghost opened this issue May 24, 2021 · 4 comments

Comments

@ghost
Copy link

ghost commented May 24, 2021

I've been trying to use SNAP in containers lately. The PyroSAR interface is quite handy. However, there is no need to install the entire Jupyter stack with all its dependencies. It really bloats the container.

I saw the following issue in the PyroSAR repository: johntruckenbrodt/pyroSAR#140

What needs to be done to make this split of dependencies happen?

Is the following modification of setup.py all that is needed?

    install_requires=[
        "progressbar2",
-       "jupyter",
-       "IPython",
-       "ipywidgets",
-       "matplotlib",
        "pathos>=0.2",
        "numpy",
        "tblib",
        "pyyaml",
        "dill",
    ],
+   extras_requires={
+       "visualization": ["IPython", "ipywidgets", "matplotlib", "jupyter"],
+       "testing": ["pytest"],
+   },

I am new to Python packages, and I'm using Python Poetry to handle my own dependencies. Because of this, I'm not very sure of how setup.py works.

With the above modification, I am able to install PyroSAR with Python Poetry with this in my pyproject.toml

[tool.poetry.dependencies]
python = "^3.8"
spatialist = {path = "../spatialist"}
pyroSAR = "^0.12.1"

where the local path is just this git repository with the modified setup.py.

@johntruckenbrodt
Copy link
Owner

Hi @ojaace! Thanks for taking up this issue. I fully agree with you, it is completely unnecessary to install anything related to Jupyter with pyroSAR so it is a good idea to separate it in spatialist.
I gave it a shot with commit 989f28c. I think in the longer term it might be better to completely separate the packages but for now this should do the trick.
Does that work for you?

@johntruckenbrodt
Copy link
Owner

Okay, just saw that matplotlib is actually also used in another function cmap_mpl2gdal. Will fix tomorrow.

@johntruckenbrodt
Copy link
Owner

I have now re-added matplotlib with commit 302851e. For the time being this has to do and at least we don't have the jupyter packages involved anymore.
Let's leave the issue open though because there is clearly more work to be done. The current changes do not yet apply to the conda package and the spatialist.explorer module is best outsourced into a separate package.

@ghost
Copy link
Author

ghost commented May 28, 2021

Thanks a lot for looking into this! It will help a lot with the dependencies! I haven't had time to test it yet, but I expect there will be no problems.

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

No branches or pull requests

1 participant