Skip to content

Conversation

@PeterNSteinmetz
Copy link
Contributor

@PeterNSteinmetz PeterNSteinmetz commented Nov 30, 2020

This request incorporates changes to handle headers from Cheetah v5.4.0. Also better error recovery and messages when new header styles that are not understood are encountered.

This also handles the type of variability in the time gaps between records in a contiguous section which can arise when the sampling rate is quite low, like 1017 Hz.

This resolves issue #906

This will not pass tests until PR # 32 in ephy_testing_data is incorporated at gin-g.node.

Peter N. Steinmetz added 30 commits October 9, 2020 11:15
Tests for v5.5.1 still failing.
…ssignement.

Using a tolerance over a longer experiment is not sensitive enough to detect blocks where perhaps a large amount of samples are dropped and there is a small gap afterwards.
Strange this hasn’t caused issues before with other file sets.
Like files from Dr. Florian Mormann’s lab.
Peter N. Steinmetz added 14 commits November 30, 2020 11:07
# Conflicts:
#	neo/rawio/neuralynxrawio/__init__.py
#	neo/rawio/neuralynxrawio/ncssections.py
#	neo/rawio/neuralynxrawio/nlxheader.py
# Conflicts:
#	neo/test/rawiotest/test_neuralynxrawio.py
These were created by inadvertendly committed the required changes on another branch and needing to cherry-pick off that branch.

# Conflicts:
#	neo/rawio/neuralynxrawio/__init__.py
#	neo/rawio/neuralynxrawio/ncssections.py
#	neo/rawio/neuralynxrawio/nlxheader.py
#	neo/test/rawiotest/test_neuralynxrawio.py
Attempt to see what is happening with CircleCI testing.
# Conflicts:
#	neo/rawio/neuralynxrawio/ncssections.py
# Conflicts:
#	neo/test/iotest/test_neuralynxio.py
#	neo/test/rawiotest/test_neuralynxrawio.py

# Conflicts:
#	neo/test/iotest/test_neuralynxio.py
#	neo/test/rawiotest/test_neuralynxrawio.py
…etahV5.4.0

# Conflicts:
#	neo/rawio/neuralynxrawio/nlxheader.py
#	neo/test/iotest/test_neuralynxio.py
@pep8speaks
Copy link

Hello @PeterNSteinmetz! Thanks for opening this PR. We checked the lines you've touched for PEP 8 issues, and found:

Line 227:83: W504 line break after binary operator
Line 238:88: W504 line break after binary operator

@PeterNSteinmetz
Copy link
Contributor Author

Two W204 PEP8 warnings to be ignored.

@apdavison apdavison added this to the 0.10.0 milestone Mar 4, 2021
@apdavison
Copy link
Member

Note that PR # 32 in ephy_testing_data at gin-g.node was replaced by # 35

@PeterNSteinmetz
Copy link
Contributor Author

Looks like the circleci tests are failing because the test data for Cheetah 5.4.0 is not yet in the gin repository.

@samuelgarcia
Copy link
Contributor

This is strange the file is in gin:
https://gin.g-node.org/NeuralEnsemble/ephy_testing_data/src/master/neuralynx/Cheetah_v5.4.0

Note that the old PR you did on gin is not deletable.
And have been replace by another one by Julia.

@PeterNSteinmetz
Copy link
Contributor Author

Maybe the CircleCI tests need to be restarted again? That looks like the correct file. It is truncated and was committed simply using git, no annexing needed.

@JuliaSprenger
Copy link
Member

Yes, I restarted the test.

@PeterNSteinmetz
Copy link
Contributor Author

These changes seems to pass the tests locally, so I assume it has to do with some problem with integrating the files in gin and CircleCI. Looks like it is now hanging on a timeout somehow?

@PeterNSteinmetz
Copy link
Contributor Author

Looks like this is now passing. Will likely need the squash commit as well. It is becoming staler as time goes by and will now need #968 to handle the later changes in #949.

@JuliaSprenger JuliaSprenger merged commit 4cce40f into NeuralEnsemble:master Apr 10, 2021
@JuliaSprenger
Copy link
Member

I don't think this one should be affected by #949 as it does not interact with the baserawio specifications. Now I have to fix it in #968 if there's still incompatiblities that I didn't consider yet.

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.

5 participants