Skip to content

Conversation

andrewsmartin
Copy link
Contributor

No description provided.

@regadas
Copy link
Contributor

regadas commented Mar 1, 2019

@andrewsmartin so this will not be deprecated after all?

@andrewsmartin
Copy link
Contributor Author

andrewsmartin commented Mar 1, 2019

@regadas correct, it's still valid to want to read schema within scio job. We only want to deprecate the schema inference logic. Scio shouldn't be concerned with writing schemas at all, we'll leave that to TFDV.

@regadas
Copy link
Contributor

regadas commented Mar 1, 2019

@andrewsmartin yeah makes sense!

Copy link
Contributor

@brianmartin brianmartin left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍

@codecov
Copy link

codecov bot commented Mar 1, 2019

Codecov Report

Merging #1714 into master will increase coverage by 0.03%.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #1714      +/-   ##
==========================================
+ Coverage   71.61%   71.64%   +0.03%     
==========================================
  Files         175      175              
  Lines        5365     5365              
  Branches      309      424     +115     
==========================================
+ Hits         3842     3844       +2     
+ Misses       1523     1521       -2
Impacted Files Coverage Δ
...otify/scio/tensorflow/TFScioContextFunctions.scala 40% <ø> (ø) ⬆️
...y/scio/values/PairSkewedSCollectionFunctions.scala 94.91% <0%> (+3.38%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update f314eb4...c329bf9. Read the comment docs.

@regadas regadas merged commit 4d2af20 into master Mar 1, 2019
@regadas regadas deleted the andrew/schema-read branch March 1, 2019 15:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants