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

[MNT] remove soft dependency import warnings in modules and documented requirements to add these #4398

Merged
merged 7 commits into from Apr 2, 2023

Conversation

fkiraly
Copy link
Collaborator

@fkiraly fkiraly commented Mar 26, 2023

Previously, modules that contained objects with soft dependencies were raising import warnings.
This would raise a lot of warnings in crawling based utilities such as all_estimators (and cause those utilities to have a longer runtime than otherwise).

In many cases, the warnings would be raised without a clear reason visible to the user, and would confuse more than they would help - contributing to some user frustration.
The information is still raised as an error when users try to construct the estimators in question - and there cause and effect are much clearer. Therefore, I would argue that it is an improvement to get rid of the module warnings.

This PR removes both the existing warnings in estimator modules and the convention itself:

  • removed all calls to _check_soft_dependencies with warning level at module import, in estimator modules
  • removed the instruction to write such checks from all extension templates
  • removed the bullet point in the dependency handling guide that asked to add these, and rewrote adjacent content for coherence

@fkiraly fkiraly added maintenance Continuous integration, unit testing & package distribution module:base-framework BaseObject, registry, base framework labels Mar 26, 2023
@fkiraly fkiraly merged commit 4a8dc87 into main Apr 2, 2023
22 checks passed
@fkiraly fkiraly deleted the remove-import-warnings branch April 2, 2023 18:03
fkiraly added a commit that referenced this pull request May 13, 2023
…#4554)

This removes a number of remaining import warnings from module imports,
these are raised for example when running `all_estimators`.

Completes the work and dependency handling requirements from
#4398 - most of the remaining
warnings are from PR which were still in progress when #4398 was merged.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
maintenance Continuous integration, unit testing & package distribution module:base-framework BaseObject, registry, base framework
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

1 participant