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

plugins, discussion: Revise on dataProcess & dataAccess #365

Open
WenheLI opened this issue Jul 23, 2020 · 3 comments
Open

plugins, discussion: Revise on dataProcess & dataAccess #365

WenheLI opened this issue Jul 23, 2020 · 3 comments
Labels
meta Project suggestion and discussion plugin Pipcook plugin addition, bug report and changes

Comments

@WenheLI
Copy link
Collaborator

WenheLI commented Jul 23, 2020

While developing plugins, I found that the functionalities of dataProcess & dataAccess have some overlaps since they both functioned for data manipulation with different scopes (dataProcess for one sample & dataAccess for the whole dataset).

Discussed with @yorkie, we think it would be more intuitive to move all data manipulation logics to dataProcess. (This requires us to introduce two types of dataProcess -- one for global; one for local).

Any ideas/suggestions on the current revise?

@WenheLI WenheLI added plugin Pipcook plugin addition, bug report and changes meta Project suggestion and discussion labels Jul 23, 2020
@yorkie
Copy link
Member

yorkie commented Jul 23, 2020

@WenheLI Is there a way to be compatible with DataProcessType in which 1st argument is a Sample?

@WenheLI
Copy link
Collaborator Author

WenheLI commented Jul 23, 2020

interface DataProcessType extends PipcookPlugin {
  (data: Sample | UniDataset, metadata: Metadata, args: ArgsType): Promise<void>;
}

or

interface IDataProcess {
    sample?: Sample,
    dataset?: UniDataset
}

interface DataProcessType extends PipcookPlugin {
  (data: IDataProcess, metadata: Metadata, args: ArgsType): Promise<void>;
}

Two implementations that come into my mind. : ( Though I do not think they are in good shape.
IMO, two separate sub-types would be better:

interface DataProcessType extends PipcookPlugin {
  (data: Sample, metadata: Metadata, args: ArgsType): Promise<void>;
}

interface GlobalDataProcessType extends PipcookPlugin {
  (data: UniDataset, metadata: Metadata, args: ArgsType): Promise<void>;
}

@yorkie
Copy link
Member

yorkie commented Jul 23, 2020

How about this?

interface CommonDataProcessType extends PipcookPlugin {
  (data: Sample, metadata: Metadata, args: ArgsType): Promise<void>;
}
interface GlobalDataProcessType extends PipcookPlugin {
  (dataset: UniDataset, metadata: Metadata, args: ArgsType): Promise<void>;
}
type DataProcessType = CommonDataProcessType | GlobalDataProcessType;

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
meta Project suggestion and discussion plugin Pipcook plugin addition, bug report and changes
Projects
None yet
Development

No branches or pull requests

2 participants