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

Fix transformer #235

Closed
wants to merge 3 commits into from
Closed

Fix transformer #235

wants to merge 3 commits into from

Conversation

gviejo
Copy link
Collaborator

@gviejo gviejo commented Oct 2, 2024

This PR adress issues with Transformers. When creating a pipeline, the number of basis should be equal to the number of dimensions for the feature.
If the number of columns for the feature is very high, it become tedious to add basis by hand. I added multiplication by integers for the class Basis.

There was also an issue where the class AdditiveBasis was trying to get the attributes basis1 and basis2 but they were only available as _basis1 and _basis2. As a quick hack, i added two properties for AdditiveBasis that return basis 1 and basis 2.

Extra: I also added the method __len__ for Basis where it returns basis.n_basis_funcs. It's more convenient.

@gviejo gviejo requested a review from BalzaniEdoardo as a code owner October 2, 2024 21:36
@gviejo gviejo marked this pull request as draft October 2, 2024 21:42
@gviejo gviejo marked this pull request as ready for review October 11, 2024 15:02
@codecov-commenter
Copy link

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 97.01%. Comparing base (6dcd891) to head (a8c8e36).
Report is 147 commits behind head on development.

Additional details and impacted files
@@               Coverage Diff               @@
##           development     #235      +/-   ##
===============================================
- Coverage        97.24%   97.01%   -0.24%     
===============================================
  Files               20       20              
  Lines             1815     1874      +59     
===============================================
+ Hits              1765     1818      +53     
- Misses              50       56       +6     

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

@BalzaniEdoardo
Copy link
Collaborator

Thanks @gviejo, I incorporated the change on the basis attribute in the #276 PR.
I will add the __len__ method too but I am closing this since I made a lot of changes in the Basis api and included the fixes for the TransformerBasis.

Thanks starting off this process.

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