PandasTransform - Ready for review #155

rcrowe-google · 2022-07-08T23:33:50Z

This is an initial working version of the PandasTransform component. One feature not in the proposal is that it checks to see if a Beam custom component (BaseBeamComponent) is supported in the loaded version of TFX, and uses it if it is.

github-actions · 2022-07-08T23:34:07Z

Thanks for the PR! 🚀

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

rcrowe-google · 2022-07-10T00:05:46Z

There's something failing in yapf, but other than that I think this is ready for review.

casassg

You can run YAPF locally by installing pre-commit install -- https://github.com/tensorflow/tfx-addons/blob/main/CONTRIBUTING.md#development-tips

Also seems a CI test is failing, not sure why, can take a look

casassg · 2022-07-12T01:21:13Z

tfx_addons/pandas_transform/component.py

+                      statistics=statistics,
+                      transformed_examples=transformed_examples,
+                      module_file=module_file,
+                      beam_pipeline=beam.Pipeline())


nit: should we add beam_pipeline_args somehow?

They'll be added for TFX >= 1.8.0. I haven't really thought about how to add them for earlier versions.

Also I really need to add more tests and at least one example.

I added beam_pipeline_args for both the new and old versions:

beam_pipeline_args: A string with the argv options for creating a Beam pipeline.
Note that this is a string, not a list. It will be split on spaces to create
a list. If running TFX >= 1.8.0, if beam_pipeline_args are specified they will
override the pipeline beam args.

review-notebook-app · 2022-08-12T00:11:47Z

Check out this pull request on

See visual diffs & provide feedback on Jupyter Notebooks.

Powered by ReviewNB

rcrowe-google · 2022-08-13T00:01:22Z

Yapf is still complaining about something, but I can't quite see what it is.

casassg · 2022-08-17T20:58:39Z

Also, @rcrowe-google would be curious if you want to add more context to Contributing.md file. Would be nice to double validate that pre-commit stuff works fine as we get more contributors.

examples/pandas_transform/pandas_transform_example.ipynb

casassg · 2022-08-17T21:00:59Z

tfx_addons/pandas_transform/component.py

+  if not os.path.exists(module_file):
+    raise ImportError(
+        'DoPandasTransform: Module file not found: {}'.format(module_file))


nit: Should we try to check if its a GCS file and download it if so?

It looks like that requires some parsing of the URI to extract the bucket name. I'm not finding any nice clean way to do that.

to be fair, we use https://github.com/tensorflow/tfx/blob/b92695748e4ea35164f7791fc3abed0d1d4679e2/tfx/utils/io_utils.py#L38 for some internal use cases so it may work for this as well

Good idea, I sorta forgot about that whole thing.

tfx_addons/version.py

tfx_addons/pandas_transform/README.md

hanneshapke · 2022-08-18T15:36:20Z

CODEOWNERS

@@ -46,3 +46,5 @@
 # Message Exit Handler
 /tfx_addons/message_exit_handler @hanneshapke

+# PandasTransform Component
+/tfx_addons/pandas_transform @rcrowe-google


@casassg Do we need to lint for blank lines are the end of files?

we do lint for that in python

hanneshapke · 2022-08-18T15:38:56Z

tfx_addons/pandas_transform/__init__.py

+
+from tfx_addons.pandas_transform.component import PandasTransform
+
+__version__ = '0.1dev'


@rcrowe-google Doesn't the number should match the version in Release.md?

Doh! Yes, it should. Fixed. Thanks!

hanneshapke · 2022-08-18T15:42:59Z

tfx_addons/pandas_transform/component.py

+                      module_file=module_file,
+                      beam_pipeline=this_beam_pipeline)
+else:
+  logging.info('TFX < 1.8.0')


@rcrowe-google Nice pattern.

hanneshapke · 2022-08-18T15:43:20Z

tfx_addons/pandas_transform/component.py

+      statistics: tfx.dsl.components.InputArtifact[ExampleStatistics] = None,
+      module_file: tfx.dsl.components.Parameter[str] = None,
+      beam_pipeline_args: tfx.dsl.components.Parameter[str] = None) -> None:
+    """Placeholder Docstring"""


@rcrowe-google Does this need an update before merging?

I clarified it here and in the other one. I thought about turning off the linting for it, but I think explaining it is better.

hanneshapke · 2022-08-18T15:43:39Z

tfx_addons/pandas_transform/component.py

+      beam_pipeline_args: tfx.dsl.components.Parameter[str] = None,
+      beam_pipeline: BeamComponentParameter[beam.Pipeline] = None,
+  ) -> None:
+    """Placeholder Docstring"""


@rcrowe-google Does this need an update before merging?

hanneshapke · 2022-08-18T15:45:16Z

@rcrowe-google I left some questions in the PR, but all are non-blocking.

casassg · 2022-08-18T16:12:18Z

@rcrowe-google this is ready for merging, you may need to rebase to get CI fixes in main (or you can just merge and it should pick it up there)

casassg · 2022-08-18T16:12:27Z

/merge

github-actions · 2022-08-18T16:13:59Z

Merge request received from @casassg! ✅

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

github-actions · 2022-08-18T18:07:17Z

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

* 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

* 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 <gcasassaez@twitter.com> * 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 <spsayakpaul@gmail.com> * clarify usage of SavedModel Co-authored-by: Sayak Paul <spsayakpaul@gmail.com> * HFModelPusher component proposal (#174) * write HFModelPusher component proposal * Update proposals/20220823-huggingface_model_pusher.md Co-authored-by: Sayak Paul <spsayakpaul@gmail.com> * Update proposals/20220823-huggingface_model_pusher.md Co-authored-by: Sayak Paul <spsayakpaul@gmail.com> * Update proposals/20220823-huggingface_model_pusher.md Co-authored-by: Sayak Paul <spsayakpaul@gmail.com> * Update proposals/20220823-huggingface_model_pusher.md Co-authored-by: Sayak Paul <spsayakpaul@gmail.com> * Update proposals/20220823-huggingface_model_pusher.md Co-authored-by: Sayak Paul <spsayakpaul@gmail.com> * Update proposals/20220823-huggingface_model_pusher.md Co-authored-by: Sayak Paul <spsayakpaul@gmail.com> * Update proposals/20220823-huggingface_model_pusher.md Co-authored-by: Sayak Paul <spsayakpaul@gmail.com> * commit_id in the outputs, more desc on the branching Co-authored-by: Sayak Paul <spsayakpaul@gmail.com> * 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 <spsayakpaul@gmail.com> * Update tfx_addons/firebase_publisher/component.py Co-authored-by: Sayak Paul <spsayakpaul@gmail.com> * Update tfx_addons/firebase_publisher/component.py Co-authored-by: Sayak Paul <spsayakpaul@gmail.com> * Update tfx_addons/firebase_publisher/executor.py Co-authored-by: Sayak Paul <spsayakpaul@gmail.com> * Update tfx_addons/firebase_publisher/runner.py Co-authored-by: Sayak Paul <spsayakpaul@gmail.com> * Update tfx_addons/firebase_publisher/runner.py Co-authored-by: Sayak Paul <spsayakpaul@gmail.com> * Update tfx_addons/firebase_publisher/runner.py Co-authored-by: Sayak Paul <spsayakpaul@gmail.com> * 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 <spsayakpaul@gmail.com> Co-authored-by: Gerard Casas Saez <gcasassaez@twitter.com> * update HFModelPusher proposal * add space_url in the output * Update proposals/20220823-huggingface_model_pusher.md Co-authored-by: Sayak Paul <spsayakpaul@gmail.com> * Update proposals/20220823-huggingface_model_pusher.md Co-authored-by: Sayak Paul <spsayakpaul@gmail.com> * 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 <gcasassaez@twitter.com> * 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 <gcasassaez@twitter.com> * Update tfx_addons/huggingface_pusher/executor_test.py Co-authored-by: Gerard Casas Saez <gcasassaez@twitter.com> * 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 <gcasassaez@twitter.com> * 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 <robertcrowe@google.com> Co-authored-by: Gerard Casas Saez <gcasassaez@twitter.com> Co-authored-by: Kshitijaa Jaglan <29124655+deutranium@users.noreply.github.com> Co-authored-by: Jeroen Van Goey <jeroen.vangoey@gmail.com> Co-authored-by: Chansung Park <deep.diver.csp@gmail.com> Co-authored-by: Sayak Paul <spsayakpaul@gmail.com>

Initial commit

9ca64ec

rcrowe-google requested review from hanneshapke, casassg and 1025KB July 8, 2022 23:33

rcrowe-google requested a review from theadactyl as a code owner July 8, 2022 23:33

rcrowe-google removed the request for review from theadactyl July 8, 2022 23:34

github-actions bot added the needs-lgtm label Jul 8, 2022

rcrowe-google added 18 commits July 9, 2022 00:25

Fixing lint

8e6102d

More lint

871025c

Yet more lint

fbfabaf

Oops, whitespace

a0d320e

Unused imports

bc2d613

More cleanup

f73759c

Misc cleanup

575100a

Fixed a couple of things

f517bc0

Fixing astype

75059a3

Disabling unnecessary-comprehension

6959d37

Oops, wrong comprehension

d5db451

Trying to improve version handling

12b8ee6

Order of imports

2c11c0d

Removing reimport

b9a0e7b

Adding version check to tests

501dc6e

Missed the beam pipeline on the min

c9187f6

Indents on the test params

6747ce6

Lint wants the else formatted differently

e688355

casassg reviewed Jul 12, 2022

View reviewed changes

Adding an example and update to pydoc for component

50e191d

rcrowe-google added 2 commits August 12, 2022 23:15

Added beam_pipeline_args, updated README

a707891

Trying to fix lint

dce62f0

rcrowe-google changed the title ~~PandasTransform - Initial commit~~ PandasTransform - Ready for review Aug 13, 2022

rcrowe-google added 3 commits August 13, 2022 00:44

Adding dependencies

b87d793

Lint, maybe yapf fixes

d996e89

Trying to fix formatting

88c3121

casassg approved these changes Aug 17, 2022

View reviewed changes

Capping Pandas version, updating CODEOWNERS

e26467f

github-actions bot added lgtm needs-merge and removed needs-lgtm labels Aug 17, 2022

hanneshapke reviewed Aug 18, 2022

View reviewed changes

hanneshapke approved these changes Aug 18, 2022

View reviewed changes

github-actions bot added merge-ready and removed needs-merge labels Aug 18, 2022

Various updates pre-merge

5bbd92d

github-actions bot merged commit 473315a into tensorflow:main Aug 18, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

PandasTransform - Ready for review #155

PandasTransform - Ready for review #155

rcrowe-google commented Jul 8, 2022

github-actions bot commented Jul 8, 2022

rcrowe-google commented Jul 10, 2022

casassg left a comment

casassg Jul 12, 2022

rcrowe-google Jul 13, 2022

rcrowe-google Jul 13, 2022

rcrowe-google Aug 18, 2022

review-notebook-app bot commented Aug 12, 2022

rcrowe-google commented Aug 13, 2022

casassg commented Aug 17, 2022

casassg Aug 17, 2022

rcrowe-google Aug 17, 2022

casassg Aug 17, 2022

rcrowe-google Aug 18, 2022

hanneshapke Aug 18, 2022

casassg Aug 18, 2022

hanneshapke Aug 18, 2022

rcrowe-google Aug 18, 2022

hanneshapke Aug 18, 2022

hanneshapke Aug 18, 2022

rcrowe-google Aug 18, 2022

hanneshapke Aug 18, 2022

hanneshapke commented Aug 18, 2022

casassg commented Aug 18, 2022

casassg commented Aug 18, 2022

github-actions bot commented Aug 18, 2022

github-actions bot commented Aug 18, 2022


		from tfx_addons.pandas_transform.component import PandasTransform

		__version__ = '0.1dev'

PandasTransform - Ready for review #155

PandasTransform - Ready for review #155

Conversation

rcrowe-google commented Jul 8, 2022

github-actions bot commented Jul 8, 2022

rcrowe-google commented Jul 10, 2022

casassg left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

review-notebook-app bot commented Aug 12, 2022

rcrowe-google commented Aug 13, 2022

casassg commented Aug 17, 2022

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

hanneshapke commented Aug 18, 2022

casassg commented Aug 18, 2022

casassg commented Aug 18, 2022

github-actions bot commented Aug 18, 2022

github-actions bot commented Aug 18, 2022