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

Implicit dependency on seaborn when extra plotting is disabled #158

Closed
pearzt opened this issue May 2, 2024 · 4 comments · Fixed by #160
Closed

Implicit dependency on seaborn when extra plotting is disabled #158

pearzt opened this issue May 2, 2024 · 4 comments · Fixed by #160
Assignees
Labels
area-stats Issues and PRs related to Thicket's stats subpackage area-visualization Issues and PRs involving any of Thicket's provided visualizations type-bug Identifies bugs in issues and identifies bug fixes in PRs type-question Issues which are not directly requesting a bugfix or feature

Comments

@pearzt
Copy link

pearzt commented May 2, 2024

Without having seaborn installed in the (virtual) environment, import thicket fails as it tries to import seaborn:

  File "[...]/benchmarking.py", line 11, in <module>
    import thicket as tt
  File "[...]/lib/python3.11/site-packages/thicket/__init__.py", line 23, in <module>
    from .stats.display_boxplot import display_boxplot
  File "[...]/lib/python3.11/site-packages/thicket/stats/display_boxplot.py", line 7, in <module>
    import seaborn as sns
ModuleNotFoundError: No module named 'seaborn'

Workaround: Just install seaborn manually or use llnl-thicket[plotting].

Related to #137.

@ilumsden ilumsden self-assigned this May 9, 2024
@ilumsden
Copy link
Collaborator

ilumsden commented May 9, 2024

@pearzt the newest version of Thicket (2024.1.0) should resolve this. Since that release, all the re-exports that use seaborn are now protected by a try-except-else block (found here).

As a result, running import thicket will no longer produce any errors related to not having seaborn. However, if you run import thicket.stats without having seaborn installed, you will get the following message (not an error):

Seaborn not found, so skipping imports of plotting in thicket.stats
To enable this plotting, install seaborn or thicket[plotting]

One last thing to be aware of: to prevent these errors, we had to change how users import stats functions. As of this release, if you want to access the stats functions (e.g., mean, std, display_boxplot), you need to import them from thicket.stats. So, for example, if you wanted to import mean, you would do:

from thicket.stats import mean

@pearce8 we should probably make a note of this somewhere in our docs.

@ilumsden ilumsden added area-visualization Issues and PRs involving any of Thicket's provided visualizations area-stats Issues and PRs related to Thicket's stats subpackage type-bug Identifies bugs in issues and identifies bug fixes in PRs type-question Issues which are not directly requesting a bugfix or feature labels May 9, 2024
@pearzt
Copy link
Author

pearzt commented May 13, 2024

@ilumsden I can still reproduce the issue with Thicket 2024.1.0. It seems to me that there still is an "import path" that imports seaborn unconditionally:

>>> import thicket
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "[...]/lib/python3.11/site-packages/thicket/__init__.py", line 13, in <module>
    from . import (
  File "[...]/lib/python3.11/site-packages/thicket/stats/__init__.py", line 24, in <module>
    from .display_violinplot import display_violinplot_thicket
  File "[...]/lib/python3.11/site-packages/thicket/stats/display_violinplot.py", line 7, in <module>
    import seaborn as sns
ModuleNotFoundError: No module named 'seaborn'

@ilumsden
Copy link
Collaborator

@pearzt thanks for checking. I just noticed that two of the "display" modules are not being protected by the "try-except-else" guard. I'll fix that.

@ilumsden
Copy link
Collaborator

@pearzt this issue will be fixed by #160

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-stats Issues and PRs related to Thicket's stats subpackage area-visualization Issues and PRs involving any of Thicket's provided visualizations type-bug Identifies bugs in issues and identifies bug fixes in PRs type-question Issues which are not directly requesting a bugfix or feature
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants