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.

Sign up for GitHub

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

Jump to bottom

Revisit {from,to}_vineyard for tensors and dataframes. #2419

Merged
merged 15 commits into from
Sep 7, 2021
Merged

Revisit {from,to}_vineyard for tensors and dataframes. #2419

merged 15 commits into from
Sep 7, 2021

Conversation

sighingnow
Copy link
Contributor

@sighingnow sighingnow commented Sep 2, 2021 •
edited
Loading

What do these changes do?

  1. add a storage_backend_info interface
  2. implements {from,to}_vineyard ops
  3. add tests, and related cleanup.

Related issue number

N/A

@qinxuye qinxuye added this to the v0.8.0a3 milestone Sep 2, 2021
@sighingnow
Fixes issues on the CI.
aa6af4f 
Signed-off-by: Tao He <[email protected]>
@sighingnow
Fixes.
1ee9a30 
Signed-off-by: Tao He <[email protected]>
@sighingnow
Fixes.
c464f3c 
Signed-off-by: Tao He <[email protected]>
@sighingnow
Fixes for ray test.
ed759d8 
Signed-off-by: Tao He <[email protected]>
@sighingnow
Copy link
Contributor Author

This pull request is ready for review.

@qinxuye qinxuye added the to be backported Indicate that the PR need to be backported to stable branch label Sep 3, 2021
qinxuye
qinxuye reviewed Sep 3, 2021
View reviewed changes
Copy link
Collaborator

@qinxuye qinxuye left a comment

Choose a reason for hiding this comment

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

I left some comments.

mars/core/operand/base.py Outdated Show resolved Hide resolved
mars/dataframe/datasource/from_vineyard.py Outdated Show resolved Hide resolved
mars/dataframe/datasource/from_vineyard.py Outdated Show resolved Hide resolved
mars/dataframe/datastore/to_vineyard.py Outdated Show resolved Hide resolved
mars/tensor/datastore/to_vineyard.py Outdated Show resolved Hide resolved
mars/tensor/datastore/to_vineyard.py Outdated Show resolved Hide resolved
mars/dataframe/datastore/to_vineyard.py Show resolved Hide resolved
mars/dataframe/datastore/to_vineyard.py Show resolved Hide resolved
mars/services/context.py Show resolved Hide resolved
mars/storage/filesystem.py Outdated Show resolved Hide resolved
@qinxuye qinxuye modified the milestones: v0.8.0a3, v0.8.0b1 Sep 3, 2021
@sighingnow
Copy link
Contributor Author

CI turns green now.

@qinxuye
Copy link
Collaborator

qinxuye commented Sep 7, 2021

Can we increase the coverage a bit? For those branch skip test maybe add no cover.

@sighingnow
No cover.
fd9378d 
Signed-off-by: Tao He <[email protected]>
@sighingnow
Copy link
Contributor Author

Can we increase the coverage a bit? For those branch skip test maybe add no cover.

Done.

qinxuye
qinxuye approved these changes Sep 7, 2021
View reviewed changes
Copy link
Collaborator

@qinxuye qinxuye left a comment

Choose a reason for hiding this comment

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

LGTM

wjsi
wjsi approved these changes Sep 7, 2021
View reviewed changes
Copy link
Member

@wjsi wjsi left a comment

Choose a reason for hiding this comment

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

LGTM

@wjsi wjsi merged commit 7c43c8d into mars-project:master Sep 7, 2021
qinxuye pushed a commit to qinxuye/mars that referenced this pull request Sep 8, 2021
@sighingnow
Revisit {from,to}_vineyard for tensors and dataframes (mars-project#2419
f3fdad5 
)

(cherry picked from commit 7c43c8d)
@qinxuye qinxuye mentioned this pull request Sep 8, 2021
Merged
wjsi pushed a commit that referenced this pull request Sep 8, 2021
[BACKPORT] Revisit {from,to}_vineyard for tensors and dataframes (#2419…
31fa186 
…) (#2436)
@wjsi wjsi added backported already PR has been backported and removed to be backported Indicate that the PR need to be backported to stable branch labels Sep 8, 2021
@sighingnow sighingnow deleted the ht/stash-to-from branch September 13, 2021 05:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@qinxuye qinxuye qinxuye approved these changes

@wjsi wjsi wjsi approved these changes

@chaokunyang chaokunyang Awaiting requested review from chaokunyang

@fyrestone fyrestone Awaiting requested review from fyrestone

@hekaisheng hekaisheng Awaiting requested review from hekaisheng

Assignees
No one assigned
Labels
backported already PR has been backported mod: dataframe mod: tensor mod: vineyard integration type: feature New feature
Projects
None yet
Milestone
v0.8.0b1
Development

Successfully merging this pull request may close these issues.
3 participants
@sighingnow @qinxuye @wjsi