Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Turn on StructuredDataset #885

Merged
merged 4 commits into from
Mar 15, 2022
Merged

Turn on StructuredDataset #885

merged 4 commits into from
Mar 15, 2022

Conversation

wild-endeavor
Copy link
Contributor

@wild-endeavor wild-endeavor commented Mar 10, 2022

Signed-off-by: Yee Hing Tong wild-endeavor@users.noreply.github.com

TL;DR

Remove the feature gate for StructuredDataset. This means that by default pd.DataFrame will resolve to Flyte IDL's StructuredDataset type/literal instead of FlyteSchema.

See the original PR #785

Note:
If you are seeing compilation errors, please make sure you're on Propeller version v0.16.14 and Admin version v0.6.78 or later.

Type

  • Bug Fix
  • Feature
  • Plugin

Are all requirements met?

  • Code completed
  • Smoke tested
  • Unit tests added
  • Code documentation added
  • Any pending items have an associated Issue

Complete description

How did you fix the bug, make the feature etc. Link to any design docs etc

Tracking Issue

https://github.com/flyteorg/flyte/issues/

Follow-up issue

NA
OR
https://github.com/flyteorg/flyte/issues/

Signed-off-by: Yee Hing Tong <wild-endeavor@users.noreply.github.com>
Signed-off-by: Yee Hing Tong <wild-endeavor@users.noreply.github.com>
@codecov
Copy link

codecov bot commented Mar 10, 2022

Codecov Report

Merging #885 (4425149) into master (2946952) will increase coverage by 0.01%.
The diff coverage is 71.42%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #885      +/-   ##
==========================================
+ Coverage   86.47%   86.49%   +0.01%     
==========================================
  Files         230      230              
  Lines       22078    22065      -13     
  Branches     2459     2456       -3     
==========================================
- Hits        19093    19086       -7     
+ Misses       2578     2575       -3     
+ Partials      407      404       -3     
Impacted Files Coverage Δ
flytekit/configuration/internal.py 95.45% <ø> (-0.14%) ⬇️
flytekit/types/structured/__init__.py 71.42% <60.00%> (+8.92%) ⬆️
flytekit/types/structured/structured_dataset.py 94.90% <100.00%> (+1.44%) ⬆️
flytekit/configuration/__init__.py 95.01% <0.00%> (-0.06%) ⬇️
flytekit/clients/raw.py 66.78% <0.00%> (ø)
flytekit/remote/remote.py 40.78% <0.00%> (ø)
flytekit/remote/__init__.py 100.00% <0.00%> (ø)
flytekit/remote/component_nodes.py 69.33% <0.00%> (ø)

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 2946952...4425149. Read the comment docs.

Signed-off-by: Yee Hing Tong <wild-endeavor@users.noreply.github.com>
eapolinario
eapolinario previously approved these changes Mar 11, 2022
Signed-off-by: Yee Hing Tong <wild-endeavor@users.noreply.github.com>
@wild-endeavor
Copy link
Contributor Author

@eapolinario had to fix a lint. +1?

@kumare3
Copy link
Contributor

kumare3 commented Mar 12, 2022

@eapolinario had to fix a lint. +1?

Is paper mill broken?

@eapolinario
Copy link
Collaborator

@kumare3 , nbclient, one of the dependencies of the papermill plug-in is broken and it's being tracked in jupyter/nbclient#207.

@wild-endeavor wild-endeavor merged commit 0405ef2 into master Mar 15, 2022
myz540 pushed a commit to ProjectAussie/flytekit that referenced this pull request Apr 11, 2022
Signed-off-by: Yee Hing Tong <wild-endeavor@users.noreply.github.com>
Signed-off-by: Mike Zhong <mzhong@embarkvet.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants