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

Copy attributes from the original class to the proxy #6306

Merged
merged 2 commits into from
Feb 10, 2025

Conversation

betatim
Copy link
Member

@betatim betatim commented Feb 10, 2025

This makes the proxy look and feel more like the original class.

This copies several attributes, not just __name__ and makes sure the signature of the constructor looks like that of the scikit-learn class. This is useful for people who inspect it during coding (like some editors) or ipython's LogisticRegression?.

For some reason the test in test_basic_estimator.py fails because the proxy doesn't have a __name__ attribute. Trying to work out why, if someone can spot the mistake let me know

This is an alternative/extended version of #6305

This makes the proxy look and feel more like the original class.
@betatim betatim requested a review from a team as a code owner February 10, 2025 15:09
@betatim betatim requested review from csadorf and vyasr February 10, 2025 15:09
@github-actions github-actions bot added the Cython / Python Cython or Python issue label Feb 10, 2025
Copy link
Contributor

@csadorf csadorf left a comment

Choose a reason for hiding this comment

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

LGTM!

@csadorf csadorf added improvement Improvement / enhancement to an existing function non-breaking Non-breaking change labels Feb 10, 2025
@csadorf
Copy link
Contributor

csadorf commented Feb 10, 2025

This is an alternative/extended version of #6305

@betatim This looks good to me! Should we merge this instead of #6305 ?

@dantegd
Copy link
Member

dantegd commented Feb 10, 2025

/merge

@rapids-bot rapids-bot bot merged commit aa8da6b into rapidsai:branch-25.04 Feb 10, 2025
70 checks passed
@betatim betatim deleted the luxury-proxy'ing branch February 11, 2025 08:56
dantegd added a commit to dantegd/cuml that referenced this pull request Feb 17, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Cython / Python Cython or Python issue improvement Improvement / enhancement to an existing function non-breaking Non-breaking change
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants