Skip to content

Fix: Prevent AttributeError when checking for UnifiedSolver in transform function#60

Merged
CeliaBenquet merged 6 commits into
celia/test-unified-cebrafrom
ananda/utils-wrapper-fix
Apr 16, 2026
Merged

Fix: Prevent AttributeError when checking for UnifiedSolver in transform function#60
CeliaBenquet merged 6 commits into
celia/test-unified-cebrafrom
ananda/utils-wrapper-fix

Conversation

@anandawolz
Copy link
Copy Markdown

@anandawolz anandawolz commented Aug 5, 2025

The previous code accessed cebra.solver.UnifiedSolver directly in an isinstance() check.
In CEBRA versions where UnifiedSolver is not exposed as an attribute (e.g. sklearn models) of cebra.solver, this caused an AttributeError before the check could fall through to the next condition.

Change: Removed the direct attribute lookup of UnifiedSolver to prevent AttributeError in environments where UnifiedSolver is unavailable.

  • UnifiedSolver check is replaced by BaseSolver, which is already imported in a stable manner from cebra.solver.base.
  • The function now distinguishes between:
  1. BaseSolver models (expects data and label)
  2. SklearnCEBRA models (expects only data)

Comment thread cebra_lens/utils_wrapper.py Outdated
Comment thread cebra_lens/utils_wrapper.py Outdated

def transform(model, data, label, **transform_kwargs):
if isinstance(model, cebra.solver.UnifiedSolver):
if isinstance(model, BaseSolver):
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

same here, just use solver and cebra

Copy link
Copy Markdown
Member

@CeliaBenquet CeliaBenquet left a comment

Choose a reason for hiding this comment

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

You can simplify naming here, just import them as they are called, no renaming :)

@anandawolz anandawolz force-pushed the ananda/utils-wrapper-fix branch from 30a9a88 to 57d83d4 Compare August 5, 2025 13:28
Copy link
Copy Markdown
Member

@CeliaBenquet CeliaBenquet left a comment

Choose a reason for hiding this comment

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

lgtm! Just suggestion to remove the prints if not used.

Comment thread cebra_lens/utils_wrapper.py Outdated
@CeliaBenquet
Copy link
Copy Markdown
Member

Mm actually @anandawolz let's discuss the change, it is not correct as not all solvers have the labels in the transformer (only Unified).

@CeliaBenquet CeliaBenquet self-requested a review August 5, 2025 13:57
@CeliaBenquet
Copy link
Copy Markdown
Member

CeliaBenquet commented Dec 4, 2025

@anandawolz, regarding my last comment, how did you handle it?

the def transform() in that wrapper takes a label, but that will not work for other solvers because they don't take a label. Have you tested that?

@MMathisLab
Copy link
Copy Markdown
Member

@CeliaBenquet can you finalize this PR and merge?

@CeliaBenquet
Copy link
Copy Markdown
Member

@anandawolz, regarding my last comment, how did you handle it?

the def transform() in that wrapper takes a label, but that will not work for other solvers because they don't take a label. Have you tested that?

this was addressed here: 08ec240

@CeliaBenquet CeliaBenquet merged commit 94b39ae into celia/test-unified-cebra Apr 16, 2026
@MMathisLab MMathisLab deleted the ananda/utils-wrapper-fix branch April 16, 2026 15:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants