Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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
Relax docstrings #19
Relax docstrings #19
Changes from all commits
8de751c
55c2daa
f254dc8
be55af5
40a9b4b
a4279f4
dc99769
81cf696
70f03e2
224b081
624e2a3
87d9768
21fe935
52b3aa1
ac986e3
43029b5
12e9d5b
f1c2fa9
2a4b0bf
7c29271
2806653
a5d9ecc
2c627e8
d19f0b1
406559d
9427950
b430c60
f8df9c3
092d3e9
06751b4
e69bcf3
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
This file was deleted.
This file was deleted.
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.
Shouldn't be this on the roiextractors repo?
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.
It's easier to test that the action works properly when it's on the same repo.
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 don't understand, so why not have the neuroconv one here as well?
I thought that the point of having the action here instead of the repos was so we could re-use them across the organization.
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.
The action is what is centralized in
catalystneuro/.github
The workflows are what calls that action
The workflows here are 'tests' or 'demos' of the actions here for debugging purposes
We could theoretically add 'neuroconv' here as an extra 'test case'
But the actual workflows used to assess PRs should live on their respective repos (b/c no other way to add to PR requirements, show up on repo dashboards, etc)
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.
OK. I did not get that it was meant to be a test of the action from @pauladkisson response above. I think the test could be simpler (like using a simpler repo where the failures would make sense to someone that is not familiar with roiextractors) though but that's probably too much work.
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.
It's true, a dedicated test repo with simple demo cases and documentation of expected results would be nice
But you are correct it would take some effort