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

Vizier sklearn transforms and PowerTransformY refactor #3153

Draft
wants to merge 5 commits into
base: main
Choose a base branch
from

Conversation

CompRhys
Copy link
Contributor

@CompRhys CompRhys commented Dec 6, 2024

This PR implements the Vizier Transforms as sklearn transformers and then creates a base class for applying such transformers. This refactor allows us to reuse most of the code in the PowerTransformY class and is hopefully fairly extensible. In future in may be possible to upstream the implementations of the vizier sklearn transformers somewhere else.

Because the power transformer already relied on sklearn this felt like a nice pattern.

Still need to write tests for the Ax transformer classes but have written tests and double checked the behaviour of the transforms.

@facebook-github-bot facebook-github-bot added the CLA Signed Do not delete this pull request or issue due to inactivity. label Dec 6, 2024
@codecov-commenter
Copy link

codecov-commenter commented Dec 6, 2024

Codecov Report

Attention: Patch coverage is 94.04580% with 39 lines in your changes missing coverage. Please review.

Project coverage is 95.80%. Comparing base (9c58541) to head (3199964).
Report is 23 commits behind head on main.

Files with missing lines Patch % Lines
ax/modelbridge/transforms/sklearn_y.py 89.91% 37 Missing ⚠️
...modelbridge/transforms/tests/test_log_warping_y.py 98.13% 2 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #3153      +/-   ##
==========================================
+ Coverage   95.41%   95.80%   +0.38%     
==========================================
  Files         493      506      +13     
  Lines       49956    51349    +1393     
==========================================
+ Hits        47665    49193    +1528     
+ Misses       2291     2156     -135     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CLA Signed Do not delete this pull request or issue due to inactivity.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants