Skip to content
New issue

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

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

Already on GitHub? # to your account

fix(SklearnBaseEstimatorItem): Temporarily disable skops's security feature #420

Merged
merged 2 commits into from
Oct 2, 2024

Conversation

thomass-dev
Copy link
Collaborator

@thomass-dev thomass-dev commented Oct 1, 2024

Copy link
Contributor

github-actions bot commented Oct 1, 2024

Coverage Report for ./frontend

Status Category Percentage Covered / Total
🔵 Lines 83.4% 1236 / 1482
🔵 Statements 83.4% 1236 / 1482
🔵 Functions 57.14% 48 / 84
🔵 Branches 84.11% 143 / 170
File CoverageNo changed files found.
Generated in workflow #38 for commit 85377fb by the Vitest Coverage Report Action

@thomass-dev thomass-dev linked an issue Oct 1, 2024 that may be closed by this pull request
@thomass-dev thomass-dev requested a review from tuscland October 1, 2024 14:00
@thomass-dev thomass-dev changed the title fix: Temporarily disable skops's security feature used in SklearnBaseEstimatorItem fix(SklearnBaseEstimatorItem): Temporarily disable skops's security feature Oct 1, 2024
rouk1
rouk1 previously approved these changes Oct 1, 2024
Copy link
Contributor

@rouk1 rouk1 left a comment

Choose a reason for hiding this comment

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

Sounds good to me but I do not know how to build a "unsafe" estimator.

@thomass-dev thomass-dev removed the request for review from augustebaum October 1, 2024 15:13
@thomass-dev
Copy link
Collaborator Author

Please @tuscland can you take a look? Thanks.

Copy link
Member

@tuscland tuscland left a comment

Choose a reason for hiding this comment

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

Overall that's looking great, however I would like to see a test succeeding in the case of an otherwise untrusted estimator.

@thomass-dev thomass-dev merged commit 4ca60a6 into main Oct 2, 2024
3 checks passed
@thomass-dev thomass-dev deleted the skops branch October 2, 2024 09:38
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Usability issue with skops
4 participants