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

Text featurizer warning on evalml import #1017

Closed
jeremyliweishih opened this issue Aug 3, 2020 · 7 comments · Fixed by #1045
Closed

Text featurizer warning on evalml import #1017

jeremyliweishih opened this issue Aug 3, 2020 · 7 comments · Fixed by #1045
Assignees
Labels
bug Issues tracking problems with existing features.
Milestone

Comments

@jeremyliweishih
Copy link
Contributor

jeremyliweishih commented Aug 3, 2020

In each user_guide notebook - RTD produces a warning wherever evalml is first imported:

Example

The cause of this issue is get_importable_subclasses initializing every component.

@jeremyliweishih jeremyliweishih added bug Issues tracking problems with existing features. documentation Improvements or additions to documentation labels Aug 3, 2020
@dsherry dsherry changed the title RTD Warning on v0.12.0 stable Text featurizer warning on evalml import Aug 3, 2020
@dsherry dsherry removed the documentation Improvements or additions to documentation label Aug 3, 2020
@dsherry
Copy link
Contributor

dsherry commented Aug 3, 2020

Repro

import evalml

Stack trace here.

Root cause

  • get_importable_subclasses tries to initialize every component
  • This runs at import-time because we use get_importable_subclasses to generate the static lists of estimators and components
  • The text featurizer currently raises a warning if no text column names are provided the constructor

Seems like we should do what we can to avoid raising warnings at import-time.

Ideas for a fix

  1. Update the lists generated by get_importable_subclasses to be generated at runtime, not at import-time. I.e. in component utils
def _all_estimators():
    return get_importable_subclasses(Estimator, used_in_automl=False)
...
  1. Ensure none of our components raise warnings if they're constructed with default arguments. Update TextFeaturizer to not raise this particular warning. Add unit test to check for no warnings.
  2. Update get_importable_subclasses to suppress any warnings coming from component initialization

I suggest we do items 1 and 2.

@dsherry dsherry added this to the August 2020 milestone Aug 3, 2020
@jeremyliweishih jeremyliweishih self-assigned this Aug 12, 2020
@jeremyliweishih
Copy link
Contributor Author

jeremyliweishih commented Aug 12, 2020

When #1022 was merged - the runtime warning was removed. However, I'm still seeing the following warning:

2020-08-11 12:17:34,559 featuretools - WARNING Featuretools failed to load plugin nlp_primitives from library nlp_primitives. For a full stack trace, set logging to debug.

@dsherry I'll put up the PR to do items 1 and 2 as you mentioned above but @eccabay has informed me that this warning is not coming from our side.

@jeremyliweishih
Copy link
Contributor Author

After discussing with @rwedge there seems to be two solutions to the above featuretools warning:

  1. use the beta pip dependency resolver

  2. featuretools is in the process of making tensorflow optional and can be done by EOD 8/12/2020

I suggest we wait for the new featuretools release and confirm if the warning is still an issue.

@dsherry
Copy link
Contributor

dsherry commented Aug 12, 2020

Thanks @jeremyliweishih ! I agree option 2 here is best for handling the featuretools warning.

That aside, my understanding is you're currently working on options 1 and 2 from my comment from last week, correct? I think both of those changes, particularly the first, will add value here.

@rwedge
Copy link
Contributor

rwedge commented Aug 12, 2020

nlp-primitives 1.0 is out on pypi now and tensorflow is no longer a required dependency

@jeremyliweishih
Copy link
Contributor Author

Confirmed that warning does not pop up with latest version of nlp-primitives. Thanks @rwedge! You can check here.

@dsherry
Copy link
Contributor

dsherry commented Aug 14, 2020

Awesome, thanks both!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Issues tracking problems with existing features.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants