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

Add optional install options with pip #627

Merged
merged 11 commits into from
Jun 21, 2024
Merged

Conversation

SarahG-579462
Copy link
Contributor

@SarahG-579462 SarahG-579462 commented Jun 6, 2024

Types of changes

This PR allows users to install the optional dependencies found in requirements.txt with pip install POT[all], or specific requirements with pip install POT[backend-pytorch] for example.

Motivation and context / Related issue

This clarifies that requirements.txt are optional dependencies, and finds which modules are optional for specific parts of the code and adds them as an installation option.

The specific install options are: backend-jax, backend-tf, backend-torch, cvxopt, dr, gnn, all.

Using all installs all modules previously available in requirements.txt (which is renamed requirements_opt.txt)

This solves the misunderstanding I had in #622, by clarifying that cvxopt is an optional dependency.

How has this been tested (if it applies)

  • running pip install .[subparts] to ensure that the requested modules are installed

PR checklist

  • I have read the CONTRIBUTING document.
  • The documentation is up-to-date with the changes I made (check build artifacts).
  • All tests passed, and additional code has been covered with new tests.
  • I have added the PR and Issue fix to the RELEASES.md file.

@SarahG-579462 SarahG-579462 changed the title [WIP] Add optional install options with pip Add optional install options with pip Jun 6, 2024
Copy link
Collaborator

@rflamary rflamary left a comment

Choose a reason for hiding this comment

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

This is nice.

I would like tu name the re quirement file requirements_all.txt instead of
opt. Also It shoudo be used/upadted in the github actions

setup.py Outdated Show resolved Hide resolved
setup.py Outdated Show resolved Hide resolved
@rflamary
Copy link
Collaborator

Hello @SarahG-579462 ,

Thanks again for your contribution.

We plan on doing a release is a few days. It would be nice to have merged this PR that will allow better installation. Could you have aquicl look at my comment and merge the conflcts with current setup.py?

@rflamary
Copy link
Collaborator

thanks @SarahG-579462 , we still need to change the requirement file name to requirements_all.txt and change the file name in all tests and doc build. After that all tests should pass and we can merge

@SarahG-579462
Copy link
Contributor Author

thanks @SarahG-579462 , we still need to change the requirement file name to requirements_all.txt and change the file name in all tests and doc build. After that all tests should pass and we can merge

Great, thanks for your help !

Copy link

codecov bot commented Jun 21, 2024

Codecov Report

Attention: Patch coverage is 71.42857% with 4 lines in your changes missing coverage. Please review.

Project coverage is 96.66%. Comparing base (14c08ba) to head (b8e2629).

Additional details and impacted files
@@            Coverage Diff             @@
##           master     #627      +/-   ##
==========================================
- Coverage   96.68%   96.66%   -0.03%     
==========================================
  Files          85       85              
  Lines       16890    16898       +8     
==========================================
+ Hits        16330    16334       +4     
- Misses        560      564       +4     

@rflamary rflamary merged commit aa4f370 into PythonOT:master Jun 21, 2024
16 checks passed
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.

2 participants