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

sklearnserver: ignore converting instances into np.array #1972

Merged
merged 5 commits into from Jan 13, 2022

Conversation

Suresh-Nakkeran
Copy link
Contributor

@Suresh-Nakkeran Suresh-Nakkeran commented Jan 3, 2022

To support mix types of input such as pandas data frames, we remove the numpy array conversion in sklearnserver which can be done in transformer if needed.

fixes #1912

@Suresh-Nakkeran Suresh-Nakkeran force-pushed the remove-numpy-array branch 2 times, most recently from e2768cf to 156a7ce Compare January 3, 2022 14:57
@yuzisun
Copy link
Member

yuzisun commented Jan 5, 2022

@Suresh-Nakkeran Can you help test with the mixed type example with your change?

@Suresh-Nakkeran Suresh-Nakkeran force-pushed the remove-numpy-array branch 2 times, most recently from 6a421f0 to 8f9933e Compare January 7, 2022 18:38
@@ -0,0 +1,299 @@
{
Copy link
Member

@yuzisun yuzisun Jan 8, 2022

Choose a reason for hiding this comment

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

This should be moved to sklearn v1 examples

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I moved.

from sklearn.base import TransformerMixin


class DictToDFTransformer(TransformerMixin):
Copy link
Member

Choose a reason for hiding this comment

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

This file is only used for sklearn pipeline example right?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yes @yuzisun . It is used only for sklearnserver model test.

Signed-off-by: Suresh Nakkeran <suresh.n@ideas2it.com>
Signed-off-by: Suresh Nakkeran <suresh.n@ideas2it.com>
Signed-off-by: Suresh Nakkeran <suresh.n@ideas2it.com>
Signed-off-by: Suresh Nakkeran <suresh.n@ideas2it.com>
@@ -0,0 +1,299 @@
{
Copy link
Member

@yuzisun yuzisun Jan 11, 2022

Choose a reason for hiding this comment

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

@Suresh-Nakkeran I have uploaded the model to gs://kfserving-examples/models/sklearn/1.0/mixedtype/model.joblib, can you help remove the above model.joblib file(keep the repo small) and add an inferenceservice yaml example file instead ?

predictor:
sklearn:
storageUri: "gs://kfserving-examples/models/sklearn/1.0/mixedtype"
image: "suresh1233/sklearnserver:latest"
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
image: "suresh1233/sklearnserver:latest"
image: "kserve/sklearnserver:latest"

Copy link
Member

Choose a reason for hiding this comment

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

or maybe can simply remove the image field as the image is going to be published when 0.8 is released

Copy link
Contributor Author

Choose a reason for hiding this comment

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

sure @yuzisun

Copy link
Contributor Author

Choose a reason for hiding this comment

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

updated!

2. multi-datatype renamed to mixedtype

Signed-off-by: Suresh Nakkeran <suresh.n@ideas2it.com>
@yuzisun
Copy link
Member

yuzisun commented Jan 12, 2022

/retest

@yuzisun
Copy link
Member

yuzisun commented Jan 13, 2022

Thanks @Suresh-Nakkeran !

/lgtm
/approve

@kserve-oss-bot
Copy link
Collaborator

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: Suresh-Nakkeran, yuzisun

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Sklearnserver - move numpy array convert to transformer
3 participants