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

Improve transformer api #280

Open
wants to merge 83 commits into
base: development
Choose a base branch
from
Open

Conversation

BalzaniEdoardo
Copy link
Collaborator

@BalzaniEdoardo BalzaniEdoardo commented Dec 14, 2024

Small PR allowing the chaining behavior of TransformerBasis.

Before this PRs, basis methods returning self could not be chained when the basis was wrapped by the TransformerBasis class.

Old behavior

>>> import nemos as nmo
>>> from nemos.basis._basis import Basis
>>> from nemos.basis._transformer_basis import TransformerBasis
>>> transformer_basis = nmo.basis.BSplineEval(5).to_transformer()
>>> out = transformer_basis.set_input_shape(1)
>>> isinstance(out, TransformerBasis)
False
>>> isinstance(out, Basis)
True

New behavior

>>> import nemos as nmo
>>> from nemos.basis._basis import Basis
>>> from nemos.basis._transformer_basis import TransformerBasis
>>> transformer_basis = nmo.basis.BSplineEval(5).to_transformer()
>>> out = transformer_basis.set_input_shape(1)
>>> isinstance(out, TransformerBasis)
True
>>> isinstance(out, Basis)
False

Key Features

  • Decorator that processes the output of chainable basis methods, setting the method output (the updated basis object) to the _basis attribute of the transformer, and returns the transformer.
  • New tuple attribute of the TransformerBasis listing all the chainable methods of basis.
  • Run-time decoration of chainable methods in the __getattr__ when first called + caching. Run-time decorating is necessary because if set at initialization, it would create a circular reference, and result in infinite loop when pickling.

@codecov-commenter
Copy link

codecov-commenter commented Dec 14, 2024

Codecov Report

Attention: Patch coverage is 97.85714% with 6 lines in your changes missing coverage. Please review.

Project coverage is 97.09%. Comparing base (3691356) to head (51584bb).
Report is 1 commits behind head on development.

Files with missing lines Patch % Lines
src/nemos/basis/basis.py 92.30% 4 Missing ⚠️
src/nemos/basis/_basis_mixin.py 99.11% 1 Missing ⚠️
src/nemos/basis/_transformer_basis.py 98.11% 1 Missing ⚠️
Additional details and impacted files
@@               Coverage Diff               @@
##           development     #280      +/-   ##
===============================================
+ Coverage        96.13%   97.09%   +0.96%     
===============================================
  Files               34       34              
  Lines             2507     2686     +179     
===============================================
+ Hits              2410     2608     +198     
+ Misses              97       78      -19     

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

Copy link
Collaborator

@sjvenditto sjvenditto left a comment

Choose a reason for hiding this comment

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

I committed a fix to a couple typos I found, otherwise it looks good!

@BalzaniEdoardo
Copy link
Collaborator Author

BalzaniEdoardo commented Dec 17, 2024 via email

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