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

Remove matplotlib and seaborn dependencies #672

Closed
huddlej opened this issue Feb 9, 2021 · 3 comments · Fixed by #1035
Closed

Remove matplotlib and seaborn dependencies #672

huddlej opened this issue Feb 9, 2021 · 3 comments · Fixed by #1035
Labels
documentation Improvements or additions to documentation enhancement New feature or request moderate problem Requires an average amount of work

Comments

@huddlej
Copy link
Contributor

huddlej commented Feb 9, 2021

Context

Augur supports a "full" installation that installs optional dependencies including cvxopt, matplotlib, and seaborn. Although cvxopt is required for titer models to work (arguably a dependency that should be installed anyway), the plotting modules are only used in an old titer model function and two test functions in the frequency estimation module (one and two).

Description and possible solution

We should remove the two plotting modules as dependencies, move the corresponding plotting code to documentation for the titer model and frequencies (respectively), move cvxopt to the main dependency list (now that we use the latest version that has pre-built wheels), and remove the "full" installation section of the project's setup module.

The related plotting code is effectively inaccessible to anyone who would want to use it from the command line and requires advanced knowledge of Augur's inner workings to get this code working from a Python shell/notebook. Moving the code to documentation would allow us to better document these Augur components and provide inline examples of what the plot outputs should look like.

Most of the hard work in addressing this issue would be the writing of documentation and testing of the old plotting code. The easier solution would be to delete the plotting code and associated dependencies completely.

@huddlej huddlej added enhancement New feature or request documentation Improvements or additions to documentation moderate problem Requires an average amount of work labels Feb 9, 2021
@joverlee521
Copy link
Contributor

Bumping this old issue because I had to exclude the "full" installation in my branch to add evofr to the nextstrain/base. matplotlib was causing an error in the image building process after I switched to Python 3.9

Should we just move cvxopt to the main dependency list and deal with matplotlib and seaborn later?

@tsibley
Copy link
Member

tsibley commented Aug 26, 2022

More context on the usage of these, which I sifted thru before I knew about this issue 🙃

@huddlej
Copy link
Contributor Author

huddlej commented Aug 31, 2022

@rneher may have a preference about my recommended solution to this issue, but I still stand by the plan to:

  1. include cvxopt as a primary dependency
  2. drop matplotlib and seaborn as dependencies along with the "full" installation option
  3. move the plotting code from titers into documentation

Probably 1 and 2 would be fine.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation Improvements or additions to documentation enhancement New feature or request moderate problem Requires an average amount of work
Projects
No open projects
3 participants