tensorflow/tfx

How about exporting the custom property keys used in the standard component?

jeongukjae opened this issue · 2 comments

jeongukjae commented

System information

  • TFX Version (you are using): 1.13.0
  • Environment in which you plan to use the feature (e.g., Local
    (Linux/MacOS/Windows), Interactive Notebook, Google Cloud, etc..): local, google cloud
  • Are you willing to contribute it (Yes/No): yes

Describe the feature and the current behavior/state.

The current behavior is that the custom property keys used in several components are private constants. It can be better if they are exported.

I think it is enough to export them like the Evaluator (in a constants.py file in the same directory as the component module, like every executor definitions are in executor.py), but in a consistent way.

tfx/tfx/components/evaluator/constants.py

Lines 17 to 22 in a949138

# Keys for artifact (custom) properties.
ARTIFACT_PROPERTY_BLESSED_KEY = 'blessed'
ARTIFACT_PROPERTY_CURRENT_MODEL_URI_KEY = 'current_model'
ARTIFACT_PROPERTY_CURRENT_MODEL_ID_KEY = 'current_model_id'
ARTIFACT_PROPERTY_BASELINE_MODEL_URI_KEY = 'baseline_model'
ARTIFACT_PROPERTY_BASELINE_MODEL_ID_KEY = 'baseline_model_id'

Will this change the current API? How?

The following components need to be changed.

  • Pusher
  • Infra Validator
  • Statistics Gen
  • Importer

Exporting constants like following.

  • tfx.components.pusher.constants.PUSHED_KEY
  • tfx.components.pusher.constants.PUSHED_DESTINATION_KEY
  • tfx.components.pusher.constants.PUSHED_VERSION_KEY
  • ...

Who will benefit with this feature?

This will benefit the developers who developing the custom components working with the standard artifacts.

Do you have a workaround or are completely blocked by this? :

not blocked by this.

Name of your Organization (Optional)

Any Other info.

singhniraj08 commented

@jeongukjae,

I came across a similar issue #2557 which was created by us to unify the constants across all components in standard_component_specs and not to duplicate the keys again for each components. This was proposed long back, if this approach can resolve you issue, I can take it up with the team to discuss it's implementation. Thank you!

jeongukjae commented

@singhniraj08

Great! That approach can resolve this issue, so I think it is okay to close this, and I'll track that issue.