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

Skip TensorFlow dependent tests if Tensorflow is not installed #107

Merged
merged 15 commits into from
Feb 18, 2022

Conversation

dvreed77
Copy link
Contributor

Skip TensorFlow dependent tests

fixes #106

Copy link
Contributor

@thehomebrewnerd thehomebrewnerd left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not entirely sure, but I don't think you need to make any changes in test_elmo.py or test_universal_sentence_encoder.py as those filenames will also match against the strings in the skip_files list in conftest.py causing those files to get skipped entirely if tensorflow is not installed. I think.

@dvreed77 dvreed77 marked this pull request as ready for review February 18, 2022 18:53
nlp_primitives/conftest.py Outdated Show resolved Hide resolved
docs/source/changelog.rst Outdated Show resolved Hide resolved
@rwedge
Copy link
Contributor

rwedge commented Feb 18, 2022

I'm doing some similar work on featuretools right now

EvalML did some work like this previously: alteryx/evalml#2922

I think there should be a way to run the unit tests without tensorflow installed, but the tensorflow tests should run (and fail) if tensorflow is not installed

@dvreed77 dvreed77 changed the title Skip TensorFlow dependent tests Skip TensorFlow dependent tests if Tensorflow is not installed Feb 18, 2022
nlp_primitives/conftest.py Outdated Show resolved Hide resolved
nlp_primitives/tensorflow/universal_sentence_encoder.py Outdated Show resolved Hide resolved
nlp_primitives/tests/test_elmo.py Show resolved Hide resolved
@dvreed77 dvreed77 requested a review from rwedge February 18, 2022 21:40
Copy link
Contributor

@rwedge rwedge left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good

@rwedge rwedge merged commit 2f9cf16 into main Feb 18, 2022
@rwedge rwedge deleted the issue-106-skip-tensorflow-dependent-tests branch February 18, 2022 22:31
@dvreed77
Copy link
Contributor Author

Fixes #107

@gsheni gsheni mentioned this pull request Feb 28, 2022
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.

Skip Tensorflow dependent tests if Tensorflow is not installed
3 participants