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

Refactor add quick check before import tensorflow #1076

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

gogetron
Copy link
Contributor

@gogetron gogetron commented Mar 31, 2024

Summary

🎯 Purpose: Avoid tensorflow import when X is None / numpy / pandas / list data.

[ ✏️ Write your summary here. ]
When the function assert_valid_inputs is called we default to import tensorflow to check if the provided data is a tensorflow dataset. However the first tensorflow import is slow and we could quickly check for other common inputs first. By adding this quick check we can skip the tensorflow import unless it is likely that the provided data is a tensorflow dataset. The performance impact of this check is ~40ns in my laptop when the data is actually a tensorflow dataset and it is about the same performance improvement when X is None / numpy / pandas / list and tensorflow was already imported before.

Impact

🌐 Areas Affected: The assert_valid_input function is called by many modules, thus we would skip the tensorflow import for most workflows that do not need it.

Testing

🔍 Testing Done: Existing test suite.

References

📚 Include any extra information that may be helpful to reviewers of your
Pull-Request here (e.g. relevant URLs or Wiki pages that could help give
background information).

Share links, docs, or sources that facilitated your changes.

If relevant, please include additional code snippets
(and their outputs/return values) showing alternative ways to use your newly added methods.

Reviewer Notes

💡 Include any specific points for the reviewer to consider during their review.

Copy link

codecov bot commented Mar 31, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 96.15%. Comparing base (abd0924) to head (97c580d).
Report is 1 commits behind head on master.

Additional details and impacted files
@@           Coverage Diff           @@
##           master    #1076   +/-   ##
=======================================
  Coverage   96.15%   96.15%           
=======================================
  Files          74       74           
  Lines        5850     5850           
  Branches     1044     1044           
=======================================
  Hits         5625     5625           
  Misses        134      134           
  Partials       91       91           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@jwmueller jwmueller self-requested a review April 1, 2024 22:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

1 participant