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

Bottleneck should not raise a warning unless relevant function in treeple.stats is called #310

Open
adam2392 opened this issue Aug 7, 2024 · 4 comments · May be fixed by #317
Open

Bottleneck should not raise a warning unless relevant function in treeple.stats is called #310

adam2392 opened this issue Aug 7, 2024 · 4 comments · May be fixed by #317

Comments

@adam2392
Copy link
Collaborator

adam2392 commented Aug 7, 2024

These lines seem like they are ran regardless of whether or not treeple.stats is used, since they are in the global namespace.

if BOTTLENECK_AVAILABLE and DISABLE_BN_ENV_VAR not in os.environ:
    nanmean_f = bn.nanmean
    anynan_f = lambda arr: bn.anynan(arr, axis=2)
else:
    warnings.warn(
        "Not using bottleneck for calculations involvings nans. Expect slower performance."
    )
    nanmean_f = np.nanmean
    anynan_f = lambda arr: np.isnan(arr).any(axis=2)

so I get this warning consistently no matter what

/Users/adam2392/Documents/treeple/treeple/stats/utils.py:34: UserWarning: Not using bottleneck for calculations involvings nans. Expect slower performance.
  warnings.warn(

@ryanhausen do you mind submitting a PR to patch this behavior?

@ryanhausen
Copy link
Contributor

@adam2392 sure! This warning should have only shown when treeple.stats is imported? Were you seeing it at other times?

@adam2392
Copy link
Collaborator Author

adam2392 commented Aug 8, 2024

Yeah I think because we do imports in __init__.py

Screenshot 2024-08-08 at 9 43 00 AM

I would take a look at the strategy, or designs sklearn uses to soft-import optional dependencies. E.g. polars, or joblib

@adam2392
Copy link
Collaborator Author

adam2392 commented Aug 8, 2024

Just to clarify @ryanhausen rather than warning during import, it should only warn the user when the function is used

@ryanhausen
Copy link
Contributor

@adam2392 agreed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants