Skip to content

Added a method to compute data drift on a ME#170

Merged
lpenet merged 8 commits into
masterfrom
feature/dss10-ch68855-add-drift-public-api-mes
Sep 22, 2021
Merged

Added a method to compute data drift on a ME#170
lpenet merged 8 commits into
masterfrom
feature/dss10-ch68855-add-drift-public-api-mes

Conversation

@fterrazzoni

@fterrazzoni fterrazzoni commented Sep 10, 2021

Copy link
Copy Markdown
Contributor

@fterrazzoni fterrazzoni added this to the 10.0 milestone Sep 10, 2021
@shortcut-integration

Copy link
Copy Markdown

Comment thread dataikuapi/dss/modelevaluationstore.py
@lpenet
lpenet requested a review from instanceofme September 10, 2021 16:36
@lpenet
lpenet self-requested a review September 17, 2021 14:37
@lpenet

lpenet commented Sep 17, 2021

Copy link
Copy Markdown
Contributor

Playing with it, I felt quite incomfortable with the API, despite the fact that it works.
The user can not easily get the referenceId of a ME or a SMV through the API.
We discussed this with @fterrazzoni and agreed that it would be nice to also allow to pass a ModelEvaluation or a DSSTrainedPredictionModelDetails as a parameter.
It would also be nice to allow the user to access the fullId easily. But as this is an API change, I guess your opinion is required, @instanceofme

Comment thread dataikuapi/dss/ml.py Outdated
Comment thread dataikuapi/dss/modelevaluationstore.py Outdated

@lpenet lpenet left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Almost good. One last remark on the doc. Seems important as a user could get perplex, else.

Comment thread dataikuapi/dss/modelevaluationstore.py Outdated
@fterrazzoni
fterrazzoni requested a review from lpenet September 21, 2021 17:17

@lpenet lpenet left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

LGTM+tested with Python 2.7 and 3.7. Would like to enrich API doc, but will turn that in an opportunity of learning for a newcomer. :-)

@lpenet
lpenet merged commit cebcd93 into master Sep 22, 2021
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