-
Notifications
You must be signed in to change notification settings - Fork 11
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 ClusterArray class to import any kind of microstates maps #118
base: main
Are you sure you want to change the base?
Conversation
I think that addition to the API would be very welcome. My initial impression is that it should mimic the In all 5 cases, the class signature is
Where WDYT @RunKZhang ? |
@RunKZhang I fixed my proposed class signature in the previous post, I defined it as a function for some reason 🙈 |
I agree with you for the ClassArray, and |
Sorry I don't get why define it as a function. Since the |
Definitely! I just made a mistake in my original comment. prior to edit, where I wrote:
which was obviously wrong! |
OK, then let's put it as third argument, after |
Hey @RunKZhang, thanks for your contribution ! In my opinion,
What do you think about it ? Otherwise, the rest of your discussion seems good. |
Hello @vferat , I agree with your idea! As you have mentioned, there are no labels if the cluster_centers are from ICA or other algorithms, and I have to admit that your design of this feature is thoughtful. For the remaining design, I have no objection to them. Finally, I am grateful my suggestions is taken into consideration. Thank you guys so much! |
OK, then let's go with this class signature:
I think |
We touch a sensitive point here:
The solution I see is to set |
OK, that works for me. |
I picked up this PR, and started with (1) clean-up, (2) input validation on the class, (3) I/O roundtrip to FIFF. |
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.
fitted_data
and labels
are not mandatory.
- If both
fitted_data
andlabels
are provided, then it is possible to computeGEV_
. - If neither
fitted_data
norlabels
are provided, then it is not possible to usepycrostates.metrics
.
I don't see any other parameter that might need specific handling
But if we have |
It could be, but there are some cases where the process is not straight forward:
These special cases make it difficult to decide whether labels should be automatically calculated. I'm open to discussion, as I have no arguments either for or against. |
Sounds like |
Hello, this is the new feature I described in Issues, and I open a pull requests here. This is a draft of the thoughts, and not heavily tested.
Closes #117