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

Moves stats imports from top-level __init__.py to stats folder #132

Merged
merged 3 commits into from
Feb 15, 2024

Conversation

ilumsden
Copy link
Collaborator

Previously, all of our stats functionality was exported through thicket/__init__.py. This goes against user expectations from tools like SciPy, and it lead to issues with the optional dependency on seaborn.

To fix this, I have moved all the stats re-exports into thicket/stats/__init__.py. So, now, to import e.g., mean, users would use one of the following:

# Option 1
from thicket.stats import mean
# Option 2
import thicket as th
th.stats.mean(...)

Additionally, this PR wraps all the "display" imports from stats in a try-except-else block that will only import the display functionality of stats when seaborn is present. If it is not present, users will get the following printout:

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

@ilumsden ilumsden added area-deployment Issues and PRs involving Thicket's packaging and deployment area-thicket Issues and PRs involving Thicket's core Thicket datastructure and associated classes priority-high High priority issues and PRs status-ready-for-review This PR is ready to be reviewed by assigned reviewers type-feature Requests for new features or PRs which implement new features type-internal-cleanup PRs or Issues related to the structure of the codebase, directories, and refactors labels Feb 14, 2024
@ilumsden ilumsden requested a review from slabasan February 14, 2024 18:40
@ilumsden ilumsden self-assigned this Feb 14, 2024
…_.py

Disables unused imports from black/flake in __init__.py

Fixes tests

black

add stats module to display calls

black
Fixes stats calls in test_display.py

Removes thicket.vis from the top level __init__.py
@ilumsden ilumsden force-pushed the stats_import_reorganize branch from 3c98426 to 83edaee Compare February 15, 2024 21:34
@slabasan slabasan merged commit caadf3d into LLNL:develop Feb 15, 2024
4 checks passed
Yejashi pushed a commit to TauferLab/thicket that referenced this pull request Mar 6, 2024
)

* Moves stats imports from thicket/__init__.py to thicket/stats/__init__.py

* Makes thicket an explicit namespace package so that th.stats will work

* Adds re-exports of subdirectories to try to fix namespace package issues
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-deployment Issues and PRs involving Thicket's packaging and deployment area-thicket Issues and PRs involving Thicket's core Thicket datastructure and associated classes priority-high High priority issues and PRs status-ready-for-review This PR is ready to be reviewed by assigned reviewers type-feature Requests for new features or PRs which implement new features type-internal-cleanup PRs or Issues related to the structure of the codebase, directories, and refactors
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants