Skip to content

Conversation

@nordme
Copy link

@nordme nordme commented Sep 26, 2023

Adds test functions to test unify_bad_channels.

@nordme nordme marked this pull request as draft September 26, 2023 22:27
@nordme
Copy link
Author

nordme commented Sep 26, 2023

@anaradanovic Hi Ana! Let's hold off on merging this until we can chat more tomorrow :)

@nordme
Copy link
Author

nordme commented Sep 27, 2023

Ok, pytest is passing!!!

@nordme nordme marked this pull request as ready for review September 27, 2023 18:00
Comment on lines 8 to 16
@pytest.mark.parametrize(
"instance", ("raw", "epochs", "evoked", "raw_spectrum", "epochs_spectrum")
)
def test_instance_support(instance, request, evoked):
"""Tests support of different classes."""
# test unify_bads function on instance (single input, no bads scenario)
inst = _get_inst(instance, request, evoked)
inst_out = unify_bad_channels([inst])
assert inst_out == [inst]

Choose a reason for hiding this comment

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

can be removed, only need to test one instance type.

m_out = unify_bad_channels([one_bad, no_bad, three_bad])
assert len(m_out) == 3
correct_bads = [chns[1], chns[0], chns[2]]
for i in np.arange(len(m_out)):

Choose a reason for hiding this comment

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

Suggested change
for i in np.arange(len(m_out)):
for i in range(len(m_out)):

Co-authored-by: Daniel McCloy <dan@mccloy.info>
@nordme
Copy link
Author

nordme commented Sep 27, 2023

@anaradanovic Hi Ana! Ok, Dan thinks that we're ready to accept this PR.

@nordme
Copy link
Author

nordme commented Sep 27, 2023

Ok, now, actually for real we're ready to accept this PR!

@drammock
Copy link

indeed! @anaradanovic please merge this one, then we'll do a final review/merge of the combined work

@anaradanovic anaradanovic merged commit 2fa2fdf into anaradanovic:unifying_bads Sep 28, 2023
@nordme nordme deleted the unifych branch September 28, 2023 20:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants