Closed hanneshapke closed 1 year ago
Thanks for the PR! :rocket:
Instructions: Approve using /lgtm
and mark for automatic merge by using /merge
.
@codesue Let's chat async what to do with https://github.com/tensorflow/model-card-toolkit/blob/master/model_card_toolkit/utils/tfx_util.py and the related tests. Are there reasons for it to stay in the MCT repo?
I think it should probably stay. It is used in core and it seems to contain the connector pieces to the TFX libraries, not the component par se.
Check out this pull request on
See visual diffs & provide feedback on Jupyter Notebooks.
Powered by ReviewNB
Let's chat async what to do with https://github.com/tensorflow/model-card-toolkit/blob/master/model_card_toolkit/utils/tfx_util.py and the related tests.
This might be good to consider as part of https://github.com/tensorflow/model-card-toolkit/discussions/228. I wonder if it's feasible to move functionality specific to pipelines and mlmd to the component's new home and the mlmd client library respectively but keep functions that only used tfma, tfdv, etc. If so, we could make tfx and mlmd utils available through a [tfx]
extra, for example.
/lgtm
Approval received from @codesue! :white_check_mark:
PR is approved. Missing merge command to auto-merge PR!
Thank you @codesue
/merge
Merged with approvals from codesue and rcrowe-google - thanks for the contribution! :tada:
Addresses #82
CODEOWNER