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

Save extension class info. #2585

Merged
merged 9 commits into from
Mar 29, 2024

Conversation

samuelgarcia
Copy link
Member

No description provided.

@@ -476,3 +477,24 @@ def normal_pdf(x, mu: float = 0.0, sigma: float = 1.0):
"""

return 1 / (sigma * np.sqrt(2 * np.pi)) * np.exp(-((x - mu) ** 2) / (2 * sigma**2))


def get_class_info(a_class):
Copy link
Collaborator

@h-mayorquin h-mayorquin Mar 15, 2024

Choose a reason for hiding this comment

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

I strongly suggest against something as vague as get_class_info for the name of a function. If I remember well, the point of storing these attributes is to store information about how to instantiate the class. From that perspective I think that something like:

  • get_importing_provenance
  • `get_class_location_within_package"
  • retrieve_class_path

or a combination of them.

I like retrieve_importing_provenance because I think it describes well what it does and it also does not add more suggestions to the get methods that are more used for user relevant information (e.g. get_traces).

Copy link
Member Author

Choose a reason for hiding this comment

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

Totally agree.
THanks you for the feedback. lets go for retrieve_importing_provenance

src/spikeinterface/core/core_tools.py Show resolved Hide resolved
src/spikeinterface/core/sortinganalyzer.py Outdated Show resolved Hide resolved
@alejoe91 alejoe91 added the core Changes to core module label Mar 18, 2024
Copy link
Collaborator

@h-mayorquin h-mayorquin left a comment

Choose a reason for hiding this comment

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

I think this is a good idea. This looks good to me.

@alejoe91 alejoe91 merged commit 5a77dd7 into SpikeInterface:main Mar 29, 2024
11 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
core Changes to core module
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants