filter component commit #158

vulkomilev · 2022-08-09T08:32:21Z

Fixes #<issue_number_goes_here>

It's a good idea to open an issue first for discussion.

Tests pass
Appropriate changes to README are included in PR

github-actions · 2022-08-09T08:32:39Z

Thanks for the PR! 🚀

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

casassg · 2022-08-15T17:40:40Z

tfx_addons/example_filter/component.py

+               statistics: types.BaseChannel,
+               schema: types.BaseChannel,
+               exclude_splits: Optional[List[str]] = None,
+               filter_function : () = lambda x :x ):


I'm having a bit of trouble understanding how this filter_function is used in executor, seems it's not passed into the spec?

I have pushed a commit with the fix

casassg · 2022-08-15T17:41:44Z

tfx_addons/example_filter/component.py

+
+  See [the ExampleValidator
+  guide](https://www.tensorflow.org/tfx/guide/exampleval) for more details.
+  """


This seems to be a description for ExampleValidator component whats the usage and description for ExampleFilter?

casassg · 2022-08-15T17:42:22Z

tfx_addons/example_filter/executor.py

+_TELEMETRY_DESCRIPTORS = ['Evaluator']
+
+
+class Executor(base_beam_executor.BaseBeamExecutor):


Having a hard time figuring out how is filtering happenning? Seems there's a lot of code here from model evaluator, can we remove extraneous code?

I have pushed a commit with the fix

casassg · 2022-10-07T16:41:36Z

tfx_addons/example_filter/component.py

+    """
+
+    SPEC_CLASS = standard_component_specs.Filter_FunctionSpec
+    EXECUTOR_SPEC = executor_spec.BeamExecutorSpec(executor.Executor)


You are using ExampleValidator Executor

casassg · 2022-10-07T16:41:56Z

tfx_addons/example_filter/component.py

+    guide](https://www.tensorflow.org/tfx/guide/exampleval) for more details.
+    """
+
+    SPEC_CLASS = standard_component_specs.Filter_FunctionSpec


This spec class is incorrect as it doesnt exist in TFX

casassg · 2022-10-07T16:42:10Z

tfx_addons/example_filter/component.py

+class ExampleFilter(base_beam_component.BaseBeamComponent):
+    """A TFX component to validate input examples.
+
+    The ExampleValidator component uses [Tensorflow Data


Documentation is for ExampleValidator not ExampleFilter

casassg · 2022-10-07T16:42:45Z

tfx_addons/example_filter/component.py

+                 baseline_model: Optional[types.BaseChannel] = None,
+                 feature_slicing_spec: Optional[Union[evaluator_pb2.FeatureSlicingSpec,
+                                                      data_types.RuntimeParameter]] = None,
+                 fairness_indicator_thresholds: Optional[Union[
+                     List[float], data_types.RuntimeParameter]] = None,
+                 example_splits: Optional[List[str]] = None,
+                 eval_config: Optional[tfma.EvalConfig] = None,
+                 schema: Optional[types.BaseChannel] = None,
+                 module_file: Optional[str] = None,
+                 exclude_splits: Optional[List[str]] = None,
+
+                 module_path: Optional[str] = None,


Why is there a baseline_model argument? Same for fairness_indicator, etc.

casassg · 2022-10-07T16:45:13Z

tfx_addons/example_filter/executor.py

+                        tensor_adapter_config=tensor_adapter_config)
+
+                try:
+                    examples_list = filter(input_dict[standard_component_specs.FILTER_FUNCTIONS_KEY][0], examples_list)


This won't work. examples_list contains a few Beam pipelines not examples itself.

casassg · 2022-10-07T16:46:01Z

tfx_addons/example_filter/executor.py

+            blessing.set_string_custom_property(
+                'component_id', exec_properties['current_component_id'])
+        # Check validation result and write BLESSED file accordingly.
+        logging.info('Checking validation results.')


Why is exampleFilter doing model validation? Let's remove anything that is not related to the example filter.

review-notebook-app · 2022-10-12T15:40:05Z

Check out this pull request on

See visual diffs & provide feedback on Jupyter Notebooks.

Powered by ReviewNB

casassg

why is there a new ipynb?

casassg · 2022-10-12T16:03:56Z

tfx_addons/example_filter/component.py

+
+
+# Output artifact containing required data related to feature selection
+class FeatureSelectionArtifact(artifact.Artifact):


whats this for?

casassg · 2022-10-12T16:04:13Z

tfx_addons/example_filter/component.py

+  print('raw_dataset')
+  print(raw_dataset)


remove print

casassg · 2022-10-12T16:04:45Z

tfx_addons/example_filter/component.py

+  ]
+  return file_list
+# reads and returns data from TFRecords at URI as a list of dictionaries with values as numpy arrays
+def _get_data_from_tfrecords(train_uri):


missing type annotations

casassg · 2022-10-12T16:05:06Z

tfx_addons/example_filter/component.py

+  return [feature_keys, target, input_data]
+
+# reads and returns data from TFRecords at URI as a list of dictionaries with values as numpy arrays
+def _get_data_from_tfrecords(train_uri):


this function is defined twice?

casassg · 2022-10-12T16:05:30Z

tfx_addons/example_filter/component.py

+        filter_function_str : Parameter[str],
+        filtered_data: OutputArtifact[standard_artifacts.Examples],
+) -> OutputDict(list_len=int):
+  '''My simple trainer component.'''


let's add documentation on how to use the component

casassg · 2022-10-12T16:06:00Z

tfx_addons/example_filter/component_test.py

+import tensorflow as tf
+from absl.testing import absltest
+from tfx.types import artifact_utils, channel_utils, standard_artifacts
+import component


use relative imports or import from tfx_addons import path

still not fixed ^

still not fixed

casassg · 2022-10-12T16:07:51Z

tfx_addons/example_filter/filter_function.py

@@ -0,0 +1,4 @@
+def filter_function(x_list):


Should this go under a folder?

casassg · 2022-10-12T16:13:40Z

Also generally, why not use a Beam pipeline to do so vs this numpy filter?

casassg · 2022-10-26T20:26:24Z

tfx_addons/example_filter/component.py

+
+
+@component
+def MyTrainerComponent(


Is the component name updated?

casassg · 2022-10-26T20:27:22Z

tfx_addons/example_filter/component.py

+
+
+# returns data in list and nested list formats compatible with sklearn
+def _data_preprocessing(np_dataset, target_feature):


is this used?

casassg · 2022-10-26T20:27:45Z

tfx_addons/example_filter/component.py

+# get a list of files for the specified path
+def _get_file_list(dir_path):
+    file_list = [
+        f for f in os.listdir(dir_path)


does this work when dir_path is a GCS path?

casassg · 2022-10-26T20:28:00Z

tfx_addons/example_filter/component.py

+
+
+# update example with selected features
+def _update_example(selected_features, orig_example):


is this used?

casassg · 2022-10-26T20:30:06Z

tfx_addons/example_filter/component.py

+    records = _get_data_from_tfrecords(input_data.uri + "/Split-train")
+    filter_function = importlib.import_module(filter_function_str).filter_function
+    records = filter_function(records)
+    filtered_data = records


filtered_data is an artifact, you need to save the data out as tf.example into filtered_data.uri. Check https://www.tensorflow.org/tfx/tutorials/tfx/python_function_component for how to do so.

casassg · 2022-10-26T20:32:12Z

tfx_addons/example_filter/labels.py

+EXTERNAL_CONFIG_VERSION = 'external_config_version'
+
+# Output labels
+SCHEMA_DIFF_PATH = 'schema_diff_path'


is this used?

casassg · 2022-10-26T20:32:19Z

tfx_addons/example_filter/spec.py

+SAMPLER_SAMPLE_KEY = 'sampling_strategy'
+
+
+class SamplingStrategy(enum.IntEnum):


Is this used?

casassg · 2022-10-26T20:32:55Z

tfx_addons/example_filter/executor.py

+# See the License for the specific language governing permissions and
+# limitations under the License.
+"""Generic TFX model evaluator executor."""
+


Is this used?

casassg · 2022-10-26T20:33:10Z

tfx_addons/example_filter/component_test.py

+
+        }
+
+        component.ExampleFilter(**params)


Can we add a teest to ensure that logic gets executed?

casassg · 2022-10-26T20:33:22Z

tfx_addons/example_filter/component_test.py

+import tensorflow as tf
+from absl.testing import absltest
+from tfx.types import artifact_utils, channel_utils, standard_artifacts
+import component


still not fixed ^

rcrowe-google · 2022-11-01T21:33:40Z

I don't understand. Iris_example_colab.ipynb is the example notebook from the feature selection component. Why is it included here?

vulkomilev · 2022-11-02T21:05:19Z

I don't understand. Iris_example_colab.ipynb is the example notebook from the feature selection component. Why is it included here?

it will be removed.

tfx_addons/example_filter/Iris_example_colab.ipynb

tfx_addons/example_filter/component.py

casassg · 2023-02-01T17:38:54Z

tfx_addons/example_filter/component_test.py

+import tensorflow as tf
+from absl.testing import absltest
+from tfx.types import artifact_utils, channel_utils, standard_artifacts
+import component


still not fixed

casassg · 2023-02-01T17:40:14Z

tfx_addons/example_filter/gcp.py

@@ -0,0 +1,250 @@
+# Copyright 2021 Google LLC. All Rights Reserved.


Would suggest instead to write one without GCP which just runs locally. There's a lot of boilderplate here which is not needed

vulkomilev · 2023-02-17T19:19:22Z

made a new push @casassg

hanneshapke · 2023-04-13T16:08:40Z

@vulkomilev Here are the steps to finish the component PR:

Update copy right in all files of your component
Update tests, make sure they run and remove anything Pandas Transform related
run the linter and make sure that the linter is green on Github!
update the example notebook with an example csv file, load the csv data, and apply the filter component, and print the content of the TFRecords (the first 4 samples)
resolve all conversations above which you have addressed, leave the ones open for which you have questions. That helps to quickly review the PR.

Thank you!

filter component commit

8143dd6

github-actions bot added the needs-lgtm label Aug 9, 2022

casassg requested changes Aug 15, 2022

View reviewed changes

v1.0 of filter component

376f519

casassg requested changes Oct 7, 2022

View reviewed changes

updated filter component with @component

89dd14f

casassg reviewed Oct 12, 2022

View reviewed changes

fixed imports

d0ef78b

casassg requested changes Oct 26, 2022

View reviewed changes

some fixes in anwser to the comments

6dc25c5

vulkomilev requested review from rcrowe-google and theadactyl as code owners November 1, 2022 18:22

rcrowe-google removed the request for review from theadactyl November 1, 2022 21:24

v1.0 of filter component

1895c72

casassg requested changes Feb 1, 2023

View reviewed changes

fixed imports

5927604

vulkomilev added 6 commits April 19, 2023 21:35

stable v1

221f31f

some lint fixes

2a85c39

some lint fixes

df2219f

some lint fixes

ce383fa

some lint fixes

db6765a

some lint fixes

b0756bc

vulkomilev closed this by deleting the head repository Apr 25, 2023

		_TELEMETRY_DESCRIPTORS = ['Evaluator']


		class Executor(base_beam_executor.BaseBeamExecutor):



		# Output artifact containing required data related to feature selection
		class FeatureSelectionArtifact(artifact.Artifact):



		# returns data in list and nested list formats compatible with sklearn
		def _data_preprocessing(np_dataset, target_feature):



		# update example with selected features
		def _update_example(selected_features, orig_example):

		SAMPLER_SAMPLE_KEY = 'sampling_strategy'


		class SamplingStrategy(enum.IntEnum):

		@@ -0,0 +1,250 @@
		# Copyright 2021 Google LLC. All Rights Reserved.

filter component commit #158

filter component commit #158

Conversation

vulkomilev commented Aug 9, 2022

github-actions bot commented Aug 9, 2022

Choose a reason for hiding this comment

vulkomilev Oct 7, 2022 • edited

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

review-notebook-app bot commented Oct 12, 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

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

casassg commented Oct 12, 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

rcrowe-google commented Nov 1, 2022

vulkomilev commented Nov 2, 2022

Choose a reason for hiding this comment

Choose a reason for hiding this comment

vulkomilev commented Feb 17, 2023

hanneshapke commented Apr 13, 2023

vulkomilev Oct 7, 2022 •

edited