-
Notifications
You must be signed in to change notification settings - Fork 45
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
Extend probe constructor #206
Changes from 5 commits
d4951e7
886e7b1
f56a9d7
a61710e
cc9eee1
4b4b906
c7a83de
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -13,7 +13,7 @@ def test_NP2(): | |
# NP2 | ||
probe = read_openephys(data_path / "OE_Neuropix-PXI" / "settings.xml") | ||
assert probe.get_shank_count() == 1 | ||
assert "2.0 - Single Shank" in probe.annotations["name"] | ||
assert "2.0 - Single Shank" in probe.model_name | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. For me name of a probe is not the model name. A name can be something which is mainingfull for a end user : like "probeA" There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. in fact this is now "model_name". The model name is the manufacturer name of the probe. Name can be anything (like ProbeA) |
||
|
||
|
||
def test_NP1_subset(): | ||
|
@@ -23,15 +23,15 @@ def test_NP1_subset(): | |
) | ||
|
||
assert probe_ap.get_shank_count() == 1 | ||
assert "1.0" in probe_ap.annotations["name"] | ||
assert "1.0" in probe_ap.model_name | ||
assert len(probe_ap.contact_positions) == 200 | ||
|
||
probe_lf = read_openephys( | ||
data_path / "OE_Neuropix-PXI-subset" / "settings.xml", stream_name="ProbeA-LFP" | ||
) | ||
|
||
assert probe_lf.get_shank_count() == 1 | ||
assert "1.0" in probe_lf.annotations["name"] | ||
assert "1.0" in probe_lf.model_name | ||
assert len(probe_lf.contact_positions) == 200 | ||
|
||
# Not specifying the stream_name should raise an Exception, because both the ProbeA-AP and | ||
|
@@ -47,7 +47,7 @@ def test_multiple_probes(): | |
) | ||
|
||
assert probeA.get_shank_count() == 1 | ||
assert "1.0" in probeA.annotations["name"] | ||
assert "1.0" in probeA.model_name | ||
|
||
probeB = read_openephys( | ||
data_path / "OE_Neuropix-PXI-multi-probe" / "settings.xml", | ||
|
@@ -69,10 +69,10 @@ def test_multiple_probes(): | |
|
||
assert probeD.get_shank_count() == 1 | ||
|
||
assert probeA.annotations["probe_serial_number"] == "17131307831" | ||
assert probeB.annotations["probe_serial_number"] == "20403311724" | ||
assert probeC.annotations["probe_serial_number"] == "20403311714" | ||
assert probeD.annotations["probe_serial_number"] == "21144108671" | ||
assert probeA.serial_number == "17131307831" | ||
assert probeB.serial_number == "20403311724" | ||
assert probeC.serial_number == "20403311714" | ||
assert probeD.serial_number == "21144108671" | ||
|
||
probeA2 = read_openephys( | ||
data_path / "OE_Neuropix-PXI-multi-probe" / "settings_2.xml", | ||
|
@@ -89,7 +89,7 @@ def test_multiple_probes(): | |
) | ||
|
||
assert probeB2.get_shank_count() == 1 | ||
assert "2.0 - Multishank" in probeB2.annotations["name"] | ||
assert "2.0 - Multishank" in probeB2.model_name | ||
|
||
ypos = probeB2.contact_positions[:, 1] | ||
assert np.min(ypos) >= 0 | ||
|
@@ -103,10 +103,11 @@ def test_older_than_06_format(): | |
) | ||
|
||
assert probe.get_shank_count() == 4 | ||
assert "2.0 - Multishank" in probe.annotations["name"] | ||
assert "2.0 - Multishank" in probe.model_name | ||
ypos = probe.contact_positions[:, 1] | ||
assert np.min(ypos) >= 0 | ||
|
||
|
||
if __name__ == "__main__": | ||
test_NP1_subset() | ||
test_multiple_probes() | ||
test_older_than_06_format() |
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 am not sure to like the idea of overloading the constructor with fixed annotation even if they are important.
When creating a Probe from scratch (wich a end user normally do not do) I think I prefer this:
The discussion of which annotations are important are endless.
So lets put annotations outside the constructor.
The other solution would be to put all of then (but I do not like)
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.
or another solution would be to make
set_*
for these 4 key annotations, so that they are still annotations (i.e., saved to file too), but the have an easier API. Whaat do you think?