Skip to content

Conversation

@PeterNSteinmetz
Copy link
Contributor

This provides refactoring of parsing contiguous blocks of records in Ncs files and handles earlier Ncs file styles. It moves the helper classes into separate files within a package sub-directory, as the main IO file had become quite large.

This replaces the old pull requests #888, #890, #891 with a single branch of the new 0.9.0 master. Those old pull requests have been deleted.

This likely obviates the need for pull request #881.

It is likely easier to understand this as a new set of files rather than each commit which went into the old pull requests gradually refactoring and moving these items.

Peter N. Steinmetz added 30 commits November 16, 2020 17:23
# Conflicts:
#	neo/rawio/neuralynxrawio.py
Tests for v5.5.1 still failing.

# Conflicts:
#	neo/rawio/neuralynxrawio.py
# Conflicts:
#	neo/rawio/neuralynxrawio.py
# Conflicts:
#	neo/rawio/neuralynxrawio.py
…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.
# Conflicts:
#	neo/test/rawiotest/test_neuralynxrawio.py
# Conflicts:
#	neo/rawio/neuralynxrawio.py
# Conflicts:
#	neo/test/rawiotest/test_neuralynxrawio.py
Tests for v5.5.1 still failing.
@samuelgarcia
Copy link
Contributor

About the submodules for neuralynx I would have the reflex to keep single file but if it is becoming too big to be easy to read the subfolder is ok.
easy to read is more or more the dogma in python no ?

@PeterNSteinmetz
Copy link
Contributor Author

Hi @JuliaSprenger. I resisted breaking up the file for some time, but it had become about 1500 lines long and rather awkward to use, so I followed the suggestion in the Google NeuralEnsemble group to do so.

The reasons for this are that the code to handle both the variations in treatment of the sampling frequency by Neuralynx over the years and the variations in the style of the headers used (which is technically free form text per the Neuralynx docs) by both Neuralynx and other producers of these files, such as Pegasus and my lab, is more complicated than what was in the original set of test files used during development. Handling the case of "gaps" and how this will result in additional segments, or not, in future modifications will also require more complicated algorithms.

I think breaking this up is, as @samuelgarcia notes, easier to read, particularly for those who want to understand how this is working and/or adapt it to other uses. At the same this preserves the ability for a typical user to simply use a NeuralynxRawIO object as usual.

@PeterNSteinmetz
Copy link
Contributor Author

PeterNSteinmetz commented Nov 19, 2020

Once this PR #205 is merged, I will create a pull request for the changes to address #906.

Peter N. Steinmetz added 6 commits November 20, 2020 17:05
# 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.
Not quite sure how this test was passing in the original pull request. But this skips it for now.
@PeterNSteinmetz
Copy link
Contributor Author

8 discussion points remaining which as far as I am concerned can be closed. Checked in changes to address the other points. All tests non-excluded tests passing on my end.

@PeterNSteinmetz
Copy link
Contributor Author

Appears the circleCI tests timed out after 10 mins. I don't understand that as the neuralynx and nixio tests run fine now on my machine and the test which it is complaining about timing out at the end run in about 7 seconds.

Attempt to see what is happening with CircleCI testing.
@samuelgarcia
Copy link
Contributor

Hi Peter and Julia.
Circle have maybe change the timeout policy.
Should I merge this anyway now ?

@PeterNSteinmetz
Copy link
Contributor Author

I would be reluctant to merge this without figuring out why this is failing in Circle, but not on other machines. A lot of changes in this one and it is stalling on the Neuralynx io.

Can others please pull this and check it out and see if all the tests run on their machine?

@samuelgarcia
Copy link
Contributor

I have push the button "rerun test" and now it pass tests.
I think it had nothing to do with this PR. No ?
i will try a test locally.

@samuelgarcia
Copy link
Contributor

The neuralynx io pass test on my desktop.
Tell me when I can merge.

@samuelgarcia
Copy link
Contributor

oups.
I speak too fast on the PR #896 there is also a problem with neuralynx tests.

@JuliaSprenger
Copy link
Member

After some travis issues, tests are passing now.
Thanks for your work. I will merge this now.

@JuliaSprenger JuliaSprenger merged commit 63093d2 into NeuralEnsemble:master Nov 25, 2020
jpgill86 added a commit to jpgill86/python-neo that referenced this pull request Jan 14, 2021
NeuralynxRawIO was reorganized into a multi-file module in a separate directory with NeuralEnsemble#905. This old file is not needed and may be interfering with Sphinx autodoc.
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.

4 participants