Skip to content

Conversation

@aditya0by0
Copy link
Member

This PR updates the model registration logic to use Python's built-in cls.__name__ instead of a custom cls.NAME attribute when checking and storing class references in _MODEL_REGISTRY.

🔍 Reason for the change:

  • Using cls.__name__ is more robust and Pythonic—it ensures every class is uniquely identified by its actual class name without relying on the presence or consistency of a manually defined NAME attribute.
  • Prevents potential issues where cls.NAME may be missing, misdefined, or inconsistent across subclasses.

This should make subclass registration safer and eliminate silent bugs due to missing or duplicate NAME attributes.

@aditya0by0 aditya0by0 requested a review from sfluegel05 July 3, 2025 10:07
@aditya0by0 aditya0by0 self-assigned this Jul 3, 2025
@aditya0by0 aditya0by0 added bug:fix enhancement New feature or request priority: high and removed bug:fix labels Jul 3, 2025
@sfluegel05
Copy link
Collaborator

Can you tell me why we need this name in the first place? It is saved to a _MODEL_REGISTRY, and then? Is this registry ever used?

@aditya0by0
Copy link
Member Author

Can you tell me why we need this name in the first place? It is saved to a _MODEL_REGISTRY, and then? Is this registry ever used?

The model registry is used to prevent duplicate subclasses extending ChebaiBaseNet—whether they’re defined in the same repo or in an external package like chebai-graph that builds on top of chebai.
Previously, this check relied on each subclass having an explicit NAME attribute, but this PR replaces that with cls.__name__ to make it safer and less error-prone.
Whenever a new subclass is created by extending ChebaiBaseNet, it gets registered in _MODEL_REGISTRY, so yes, the registry is actively used to ensure unique model definitions and avoid silent conflicts.

@sfluegel05
Copy link
Collaborator

Allright, then let's keep it that way.

@sfluegel05 sfluegel05 merged commit 2633810 into dev Jul 14, 2025
6 checks passed
@aditya0by0 aditya0by0 deleted the fix/implicit_model_resigtry branch July 14, 2025 17:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request priority: high

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants