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
Include ecog and seeg #281
Conversation
get_rejection_threshold
I don't really understand the error in the checks |
the error is unrelated. Maybe @hoechenberger or @sappelhoff knows what's up? |
No idea ... I was hoping this was a temporary issue. But it seems to persist. Somebody will have to dig into this :-) |
Seems like yet another issue with OpenNeuro. We'll have to |
@chapochn would you mind filing a bug report about this on https://github.com/OpenNeuroOrg/openneuro/ ? |
@sappelhoff I have no experience with OpenNeuro.
What is the command that is causing the error? |
I've field a bug report at OpenNeuroOrg/openneuro#2635 |
and apparently already solved -- I'll restart CI |
I added 2 tests, but I can see that some checks failed. I'm not sure how to see the error, do I need to be able to access Azure? I don't seem to have access. Please let me know. |
Can you just change the channel type of a regular EEG channel so as to not add a dependency on |
@@ -364,3 +364,60 @@ def test_fnirs(): | |||
assert reject["hbo"] < 0.001 # This is a very high value as sanity check | |||
assert reject["hbr"] < 0.001 | |||
assert reject["hbr"] > 0.0 | |||
|
|||
|
|||
def test_ecog(): |
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.
we follow PEP 257. See how docstrings should be formatted: https://peps.python.org/pep-0257/
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.
not sure to which place you're referring to, but I changed something, see in the next commit
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.
see: "Multi-line docstrings consist of a summary line just like a one-line docstring, followed by a blank line, followed by a more elaborate description."
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 thanks, pushed a new version
slightly changed docstring
autoreject/tests/test_autoreject.py
Outdated
epochs.load_data() | ||
n1 = len(epochs) | ||
reject = get_rejection_threshold(epochs) | ||
print(reject) |
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.
print(reject) |
no print statements in tests ...
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 copied that from a test that was already there
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.
pushed
Can you update |
ok, updated changelog, thanks for suggesting! |
looks like you need to rebase with respect to the latest master branch: $ git fetch upstream master:master
$ git rebase master
$ git push -f origin include-ecog |
Should be ok now |
I went ahead and squashed your commits. Thanks @chapochn ! 🥳 |
Great, thank you! |
the function get_rejection_threshold now also works when the data is ecog or seeg