search

tensorflow / tfx-addons

Developers helping developers. TFX-Addons is a collection of community projects to build new components, examples, libraries, and tools for TFX. The projects are organized under the auspices of the special interest group, SIG TFX-Addons. Join the group at http://goo.gle/tfx-addons-group
Apache License 2.0
125 stars 64 forks source link

Prediction to bigquery component - initial code #210

Closed hanneshapke closed 1 year ago

hanneshapke commented 1 year ago

Related to issue https://github.com/tensorflow/tfx-addons/issues/78

This is the OSS version of Digit's predictions to bigquery component.

What's missing and not part of this PR?

The component used by Digits' TFX pipelines.

github-actions[bot] commented 1 year ago

Thanks for the PR! :rocket:

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

michaelwsherman commented 1 year ago

LGTM

casassg commented 1 year ago

@hanneshapke seems the linter is failing mind fixing it up? Else new PRs may be blocked from merging https://github.com/tensorflow/tfx-addons/actions/runs/4099019498/jobs/7068535397

casassg commented 1 year ago

(also let's try to stick to using the automerge bot as bot ensures all tests/linters pass before merging avoiding issues like this)

hanneshapke commented 1 year ago

@casassg Sorry, I forgot to leave a comment about it. @michaelwsherman wanted to take over the project urgently. The component is missing the last piece (dyn schema) and it is therefore not functional. We shared it via the project, because @michaelwsherman can't fork from my dev branch.

casassg commented 1 year ago

@hanneshapke or @michaelwsherman mind adding lint ignore to all files until those get fixed? Want to avoid having to deal w issues on new PRs ideally

michaelwsherman commented 1 year ago

Echoing @hanneshapke, Google is going to finish up getting this merged in properly and adding some tests. But it may be a week or two before that's all done.

@casassg -- sorry about the rough edges here, but there was some challenges figuring out a way for Google to work on Hannes code, and getting the working code into the tfx-addons repo was the best way we could figure out how to collaborate. Would it be safer to move this "working" code into a subfolder of the repo root rather a subfolder of tfx_addons where it's in the package?

casassg commented 1 year ago

It's fine to merge, the issue is linter runs on all python files. Just adding linter-skip annotations to all new files is enough. I can do it if needed

hanneshapke commented 1 year ago

Happy to craete a fix for it in a moment

michaelwsherman commented 1 year ago

@cfezequiel to add linter-skip annotations if Hannes doesn't get to it first. Sorry folks, on the move today and hard for me to just bang this out myself.

casassg commented 1 year ago

Got it fixed up, should be fine now. Let's try to remove the linter skip annotations