-
Notifications
You must be signed in to change notification settings - Fork 156
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
Add name as an extractor attribute for __repr__
purposes
#2784
base: main
Are you sure you want to change the base?
Conversation
Hi. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I like this. I think the difference between a KilosortSorting and an NpzFolderSorting is not a giant value added if it is the same thing (also true for recordings). I would prefer the option to name mine (something like Animal1Session1Recording) that way if I'm analyzing a couple data sets I can store that information in the analysis instance and keep oriented easier.
@samuelgarcia |
Yeah, now that I think about it, I did it because some classess have that class attribute. SpikeGLX:
|
Co-authored-by: Zach McKenzie <92116279+zm711@users.noreply.github.com>
What about using When filtering etc, should the alias/name be propagated? |
For me as an end-user what I wanted to do is hack this so that I can "name"/"alias" my dataset such that it would be propagated (but I can understand why that would not want to be done). For example: recording = read_intan(xx)
recording.set_name('AnimalASession1')
rec_f = bandpass_filter(recording)
rec_f
>>> 'AnimalASession1' because for me I name my variable |
OK, from the discussion today. Yes, to propagation. It seems that we would like to use |
Here one place where we are using name: |
Let's move forward with this by:
This will make sure that it's also propagated to child objects. |
Ok @DradeAW will confirm tomorrow, but he told me he's not using So my proposal is to get rid of the Do we all agree? @h-mayorquin @samuelgarcia @zm711 We will also get rid of the |
+1 on the proposal. |
Sounds good to me! |
I can take care of this but I will wait for @samuelgarcia to move forward. |
@samuelgarcia is ok with this! We discussed yesterday :) |
All right, I will move forward with this! |
I can confirm that this PR is fine with me :) |
I am working on the tutorial for the upcoming school and I think that the following could be useful quality of live improvement for talking about notebooks.
Right now the name of an extractor object displayed by the repr in the notebooks is the name of the class:
This means that two different objects will have the same name and I can't differentiate them by their repr. A limitation when presenting notebooks and I want to talk about two different recordings of the same class.
Ti addresss this, this PR introduces another attribute
name
(but I think displayed name or id could be better?) that displayed by the repr when set (otherwise it falls back to the current default of using the class name).I also added other names to common generated objects that I think make more sense (as they don't leak implementation details).