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

Added tests for feature selection component #149

Merged

Conversation

deutranium
Copy link
Contributor

Added test for the feature_selection component.

  • Check artifact type for feature_selection artifact
  • Check artifact type for updated_data artifact

@github-actions
Copy link
Contributor

github-actions bot commented Jun 2, 2022

Thanks for the PR! 🚀

Instructions: Approve using /lgtm and mark for automatic merge by using /merge.

_data_filepath = os.path.join(_data_root, "data.csv")
urllib.request.urlretrieve(DATA_PATH, _data_filepath)

context = InteractiveContext()
Copy link
Member

Choose a reason for hiding this comment

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

InteractiveContext only works in Notebooks!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Oh okay, my bad. Would you have any resources for an alternate way? I wasn't able to find it as the examples I came across were implemented in Colab

@casassg

Copy link
Member

Choose a reason for hiding this comment

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

Use LocalDagRunner instead. xgboost and sklearn examples have a test like that

@deutranium
Copy link
Contributor Author

@casassg would this way work? Referred to xgboost and sklearn examples for the same
Thanks!

@casassg
Copy link
Member

casassg commented Aug 20, 2022

I think this looks good. Would be great if we could check the result of the component to make sure it's selecting as expected but not sure if that's tested elsewhere

@casassg
Copy link
Member

casassg commented Aug 20, 2022

/lgtm

@casassg
Copy link
Member

casassg commented Aug 20, 2022

(also note that your tests are red see the test below)

@deutranium
Copy link
Contributor Author

Thank you for your comments @casassg !

Would be great if we could check the result of the component to make sure it's selecting as expected but not sure if that's tested elsewhere

Yup, just added a test to check correctness of selected features and it seems to be running fine (locally at least)

(also note that your tests are red see the test below)

  1. I think the automerge one should be fixed once all the codeowners approve the PR
  2. I'm not so sure about the CI test though. The logs show ModuleNotFoundError: No module named 'sklearn' but how can I specify the same? I have tried adding it to requirements.txt but it doesn't seem to fix the issue

@casassg
Copy link
Member

casassg commented Aug 22, 2022

You will need to modify tfx_addons/version.py _PKG_METADATA dictionary to make sure to include what dependencies are needed for the component (key corresponds to project name, aka tfx_addons/{PROJECT_NAME}

scikit_learn==1.1.2
tensorflow
tfx
tfx_bsl==1.9.0
Copy link
Member

Choose a reason for hiding this comment

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

you can actually remove this from here as we should instead pip install -e ".[feature_selection]" (feel free to document that)

@casassg
Copy link
Member

casassg commented Aug 22, 2022

marking as ready to /merge you will need to fix the formatting issue on pre-commit check. Once fixed the PR should merge automatically

@github-actions
Copy link
Contributor

Merge request received from @casassg! ✅

PR will be auto-merged once Test suite is green!

tfx_addons/version.py Outdated Show resolved Hide resolved
tfx_addons/version.py Outdated Show resolved Hide resolved
tfx_addons/version.py Outdated Show resolved Hide resolved
@github-actions github-actions bot merged commit 5823067 into tensorflow:main Aug 22, 2022
@github-actions
Copy link
Contributor

Merged with approvals from casassg and FatimahAdwan - thanks for the contribution! 🎉

@casassg
Copy link
Member

casassg commented Aug 22, 2022

/cherry r0.3

@casassg
Copy link
Member

casassg commented Aug 22, 2022

/cherry-pick r0.3

github-actions bot pushed a commit that referenced this pull request Aug 22, 2022
* Initial commit with working tests

* cleaned and ensured the file is working with pre-commit

* Testing checkpoint

* Added support for module_file

* Improved documentation

* Fixed test

* Fixed pre-commit errors

* Added data files for component_test.py

* Added tests for artifact count by type

* Fixed minor bug

* Added test to check if correct features are being selected

* Update dependencies for feature_selection

* Update tfx_addons/version.py

* Update tfx_addons/version.py

* Update tfx_addons/version.py

Co-authored-by: Gerard Casas Saez <[email protected]>
@github-actions
Copy link
Contributor

🤖 says: cherry pick action finished successfully 🎉!
See: https://github.com/tensorflow/tfx-addons/actions/runs/2906430004

hanneshapke pushed a commit to digits/tfx-addons that referenced this pull request Apr 3, 2023
* Initial commit with working tests

* cleaned and ensured the file is working with pre-commit

* Testing checkpoint

* Added support for module_file

* Improved documentation

* Fixed test

* Fixed pre-commit errors

* Added data files for component_test.py

* Added tests for artifact count by type

* Fixed minor bug

* Added test to check if correct features are being selected

* Update dependencies for feature_selection

* Update tfx_addons/version.py

* Update tfx_addons/version.py

* Update tfx_addons/version.py

Co-authored-by: Gerard Casas Saez <[email protected]>
github-actions bot pushed a commit that referenced this pull request Apr 11, 2023
* initial move of the existing mct code

* Added missing file

* Added (unreviewed) MCT example

* updated example

* extended doc

* add census helper files

* Create 20220513-PandasTransform.md

First draft of project proposal.

* Update 20220513-PandasTransform.md

Oops, forgot to add the dependencies!

* Update 20220513-PandasTransform.md

Adding a note about statistics and schema for the changed dataset.

* Rcrowe fixing filename (#140)

* Create 20220513-pandas_transform.md

* Delete 20220513-PandasTransform.md

* Update README.md (#142)

Fixing broken links on PyPI page.

* fix bad link in xgboost_evaluator

* Update CODEOWNERS

* Cherry pick bot to backport changes (#144)

* Cherry pick bot to backport changes

* Update RELEASE.md

* validate user is release manager

* Update examples for feature selection component (#116)

* Updated module file for compatibility

* Updated module file for compatibility

* Update iris_module_file.py

* Created Palmer Penguins example using Colaboratory

* Created Pima Indians Diabetes example pipeline using Colaboratory

* Created Iris example pipeline using Colaboratory

* Deleted outdated file + replaced with new examples

* Deleted outdated file

* Fix bad link in xgboost_evaluator (#145)

Commit 93fd006 didn't correctly fix the link.

* Increase supported version from TFX <1.8 to <1.9  (#147)

* include TFX 1.8

* increase CI

* switch cache key to constaint first

* remove shared cache

* fix exit handler

* fix patch issues

* reformating yapf

* Use feast from master git to avoid pip infinite backtrack (#152)

* pin pip

* Update ci.yml

* use requirements max and min instead

* add missing letter

* fix using -r option

* ammend triggers

* add tensorflow to the mix

* tf 2.6

* move constraints to version file

* pin api-core

* add protobuf

* protobuf 3.19

* add mlp-sdk

* add extras for api-core

* add it to requirements instead

* move feast dep

* use feast repo instead of release

* remove mlpipeline-sdk

* Updated readme with module file guidelines and example (#151)

* bump feast version support (#154)

* Add feature_selection to included pkg (#161)

* add feature_selection to included pkg

* upgrade tfx to avoid beam issue in sklearn example

* fix due to upgrade of tfma

* fix missing 1 types

* format inline

* update extractor test

* remove the header generator

* break down CI by component

* use file.json

* use python file

* use correct base_dir

* touch for feast_examplegen

* ensure it works fine

* run if certain dangerous files change

* removed non used files and update CONTRIBUTING

* improve a bit how variable is set

* revert change in feastexamplegen

* remove non used variable in version.py

* rename job phase

* cancel if new ones

* add some extra space

* add note on inspiration

* PandasTransform - Ready for review (#155)

* Initial commit

* Fixing lint

* More lint

* Yet more lint

* Oops, whitespace

* Unused imports

* More cleanup

* Misc cleanup

* Fixed a couple of things

* Fixing astype

* Disabling unnecessary-comprehension

* Oops, wrong comprehension

* Trying to improve version handling

* Order of imports

* Removing reimport

* Adding version check to tests

* Missed the beam pipeline on the min

* Indents on the test params

* Lint wants the else formatted differently

* Adding an example and update to pydoc for component

* Adding README and release notes

* Added beam_pipeline_args, updated README

* Trying to fix lint

* Adding dependencies

* Lint, maybe yapf fixes

* Trying to fix formatting

* Capping Pandas version, updating CODEOWNERS

* Various updates pre-merge

* support tfx 1.9 (#162)

* small fix for filter projects (#164)

* small fix for filter projects

* Update ci.yml

* Update README to add missing component (#165)

* Update README.md

* fix pandas_transform

* Added release notes for feature selection component (#150)

* Update CONTRIBUTING.md

* Create README.md

Adding a readme to the example

* Create requirements.txt

Adding requirements.txt to the example.  Only referencing the component, since that will keep all other dependencies in one place.

* Added tests for feature selection component (#149)

* Initial commit with working tests

* cleaned and ensured the file is working with pre-commit

* Testing checkpoint

* Added support for module_file

* Improved documentation

* Fixed test

* Fixed pre-commit errors

* Added data files for component_test.py

* Added tests for artifact count by type

* Fixed minor bug

* Added test to check if correct features are being selected

* Update dependencies for feature_selection

* Update tfx_addons/version.py

* Update tfx_addons/version.py

* Update tfx_addons/version.py

Co-authored-by: Gerard Casas Saez <[email protected]>

* Improve examples CI to automatically pick up projects (#166)

* make ci_examples run only when needed

* remove non used init file

* only run those examples that have test files

* address comments and improve documentation

* add concurrency for ci-examples

* CI: Run all if event name is push (#167)

* if event name is push run all projects

* remove non-used RUN_ALL_FILE

* run ci-examples if changes in filter_examples

* add comment on why env name

* add better logging

* Move main to 0.4 after 0.3 release

* Update RELEASE.md (#170)

* update firebase publisher proposal (#171)

* update firebase publisher proposal

update firebase publisher proposal with more detailed descriptions

* remove vague word temporary

* replace custom_config with concrete parameters

* Update tfx_addons/firebase_publisher/README.md

Co-authored-by: Sayak Paul <[email protected]>

* clarify usage of SavedModel

Co-authored-by: Sayak Paul <[email protected]>

* HFModelPusher component proposal (#174)

* write HFModelPusher component proposal

* Update proposals/20220823-huggingface_model_pusher.md

Co-authored-by: Sayak Paul <[email protected]>

* Update proposals/20220823-huggingface_model_pusher.md

Co-authored-by: Sayak Paul <[email protected]>

* Update proposals/20220823-huggingface_model_pusher.md

Co-authored-by: Sayak Paul <[email protected]>

* Update proposals/20220823-huggingface_model_pusher.md

Co-authored-by: Sayak Paul <[email protected]>

* Update proposals/20220823-huggingface_model_pusher.md

Co-authored-by: Sayak Paul <[email protected]>

* Update proposals/20220823-huggingface_model_pusher.md

Co-authored-by: Sayak Paul <[email protected]>

* Update proposals/20220823-huggingface_model_pusher.md

Co-authored-by: Sayak Paul <[email protected]>

* commit_id in the outputs, more desc on the branching

Co-authored-by: Sayak Paul <[email protected]>

* fix: missing option --hook-type (#177)

fix: missing option `--hook-type` in "Install pre-commit hooks for push hooks: `pre-commit install --hook-type pre-push`"

* Scaling Sampler by using row level probability (#163)

* initial probabilistic sampling

* using sampled in batches

* use toDict

* use calculation for sampling by class

* implement using sampling at individual level

* fixup readme

* improve executor docs

* use patch to avoid flaky tests

* Add FirebasePublisher Implementation (#175)

* add empty __init__.py

* add component.py

* add component spec

* add executor.py

* unncessary codes from custom_config

* initial implementation done

* fix typo

* re-organize the runner

* pre-commit

* pre-commit

* add docstring for the component

* add basic unit tests

* add basic unit tests

* change private to public

* change function call name

* pass pre-commit

* Update tfx_addons/firebase_publisher/runner.py

Co-authored-by: Sayak Paul <[email protected]>

* Update tfx_addons/firebase_publisher/component.py

Co-authored-by: Sayak Paul <[email protected]>

* Update tfx_addons/firebase_publisher/component.py

Co-authored-by: Sayak Paul <[email protected]>

* Update tfx_addons/firebase_publisher/executor.py

Co-authored-by: Sayak Paul <[email protected]>

* Update tfx_addons/firebase_publisher/runner.py

Co-authored-by: Sayak Paul <[email protected]>

* Update tfx_addons/firebase_publisher/runner.py

Co-authored-by: Sayak Paul <[email protected]>

* Update tfx_addons/firebase_publisher/runner.py

Co-authored-by: Sayak Paul <[email protected]>

* remove mis-placed docstring in the component

* fix full-step consistency

* rename model_exist -> is_model_present

* rename upload_model -> upload_model_to_gcs

* more descriptions on update_model

* update log message on model_create

* fix case inconsistency

* fix pre-commit

* add copyright

* add import module in __init__.py

* add firebase_publisher version info

* fix typo

* remove ValueError in docstring

* update docstring

* fix: pre-commit

* fix: pre-commit

* update function name in the test module

* add copyrights

* reduce version requirement

* remove version info in __init__.py

* add constraint for firebase in CI

Co-authored-by: Sayak Paul <[email protected]>
Co-authored-by: Gerard Casas Saez <[email protected]>

* update HFModelPusher proposal

* add space_url in the output

* Update proposals/20220823-huggingface_model_pusher.md

Co-authored-by: Sayak Paul <[email protected]>

* Update proposals/20220823-huggingface_model_pusher.md

Co-authored-by: Sayak Paul <[email protected]>

* Archive penguins_sklearn example (#184)

* Update README.md

Archived and pointed to example in main TFX repo.

* Update penguin_pipeline_sklearn_gcp.py

Commenting out to avoid issues while archived.

* Update penguin_pipeline_sklearn_gcp_test.py

Commenting out to avoid CI issues while archived.

* Update penguin_pipeline_sklearn_local.py

Commenting out to avoid CI issues while archived.

* Update penguin_pipeline_sklearn_local_e2e_test.py

Commenting out to avoid CI issues while archived.

* Update penguin_pipeline_sklearn_local.py

* Update penguin_utils_sklearn.py

Commented out to avoid CI issues while archived

* Update sklearn_predict_extractor.py

Commented out to avoid CI issues while archived.

* Update sklearn_predict_extractor_test.py

Commented out to avoid CI issues while archived.

* Update requirements.txt

Commenting out to avoid CI issues while archived

* Update Dockerfile

Commenting out to avoid CI issues while archived

* Update penguin_utils_sklearn.py

Lint

* Update sklearn_predict_extractor.py

Lint

* Update sklearn_predict_extractor.py

Lint

* Update penguin_pipeline_sklearn_gcp.py

Lint

* Update penguin_pipeline_sklearn_gcp_test.py

Lint

* Update penguin_pipeline_sklearn_local.py

Lint

* Update penguin_pipeline_sklearn_local_e2e_test.py

Lint

* Update penguin_utils_sklearn.py

Lint

* Update sklearn_predict_extractor_test.py

Lint

* Update sklearn_predict_extractor_test.py

Lint

* Update penguin_utils_sklearn.py

Lint

* Update sklearn_predict_extractor.py

Lint

* Update sklearn_predict_extractor_test.py

Lint

* Update penguin_pipeline_sklearn_gcp_test.py

* Update penguin_pipeline_sklearn_local_e2e_test.py

Null test to satisfy CI

* Update sklearn_predict_extractor_test.py

Null test to satisfy CI

* Update penguin_pipeline_sklearn_gcp_test.py

Lint

* Update penguin_pipeline_sklearn_local_e2e_test.py

Lint

* Update sklearn_predict_extractor_test.py

Lint

* Update penguin_pipeline_sklearn_local_e2e_test.py

Lint

* Update penguin_pipeline_sklearn_gcp_test.py

Lint

* Update penguin_pipeline_sklearn_gcp_test.py

Lint

* Update penguin_pipeline_sklearn_local_e2e_test.py

Lint

* Update sklearn_predict_extractor_test.py

Lint

* remove files and include reference to existing code

Co-authored-by: Gerard Casas Saez <[email protected]>

* remove old data (#185)

* PyTorch TFX proposal

* clean up of CODEOWNERS (#188)

* Support TFX 1.10 (#187)

* Initial release candidate testing for tfx==1.10.0rc0

* remove suffux from test_utils

* add missing extra line

* use tfx 1.10 constraint

* add firebase_publisher to readme

* Add HF pusher owners (#192)

* Add HuggingFace Pusher Implementation (#191)

* add hfpusher implementation

* update version.py

* add test for runner.py

* linting

* add dependency

* add pytest ignore protected access

* add one more test

* fix: test failure

* copy proposa as README

* add git-lfs dependency

* document about RuntimeError when git-lfs is not installed

* add _is_git_lfs_installed

* add test cases for executor

* change _executor -> exe

* yapf

* Update tfx_addons/huggingface_pusher/README.md

Co-authored-by: Gerard Casas Saez <[email protected]>

* Update tfx_addons/huggingface_pusher/executor_test.py

Co-authored-by: Gerard Casas Saez <[email protected]>

* add huggingface document reference for git-lfs

* space_config example added to component_test

* yapf

* add optional decrypt features

* add decrypt_fn in README

Co-authored-by: Gerard Casas Saez <[email protected]>

* initial move of the existing mct code

* Added missing file

* Added (unreviewed) MCT example

* updated example

* extended doc

* add census helper files

* updated mct version dependency

* yapf + isort updates

* updated gitignore to ignore notebook generated files

* isort updated

* removed notebook example generated files

* removed model_card_pb2.SensitiveData() and model_card_pb2.ConfidenceInterval()

* updated formatting

* updated example notebook

* updated example notebook for the MCT component

* updated example notebook for the MCT component

* set the code owners for the MCT component

* Update RELEASE.md

* Update RELEASE.md

* added tfxtest

* added tfxtest

* updated .gitignore

* fix typo in CODEOWNERS

* Update .gitignore

---------

Co-authored-by: Robert Crowe <[email protected]>
Co-authored-by: Gerard Casas Saez <[email protected]>
Co-authored-by: Kshitijaa Jaglan <[email protected]>
Co-authored-by: Jeroen Van Goey <[email protected]>
Co-authored-by: Chansung Park <[email protected]>
Co-authored-by: Sayak Paul <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants