Skip to content

Conversation

@PeterNSteinmetz
Copy link
Contributor

This pull request activates the full test suite for the PRE4 file type for Cheetah version 4.0.2.

It also corrects an older off by one bug in test_neuralyxio.py which was omitting the last sample in each record during testing.

This will not run integration tests until ephy_testing_data PR#20 is merged.

Peter N. Steinmetz added 30 commits September 8, 2020 11:52
Allows full normal testing to run, using the 4.0.2 data only to test the parsing of ncs recording from the header information.
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.
@pep8speaks
Copy link

pep8speaks commented Oct 29, 2020

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

Line 540:66: W504 line break after binary operator
Line 541:72: W504 line break after binary operator
Line 542:68: W504 line break after binary operator
Line 597:32: W504 line break after binary operator
Line 639:25: E116 unexpected indentation (comment)
Line 667:82: W504 line break after binary operator
Line 780:82: W504 line break after binary operator
Line 802:45: E126 continuation line over-indented for hanging indent
Line 845:41: E126 continuation line over-indented for hanging indent
Line 851:41: E126 continuation line over-indented for hanging indent
Line 876:45: E126 continuation line over-indented for hanging indent

Comment last updated at 2020-10-29 17:51:50 UTC

@PeterNSteinmetz
Copy link
Contributor Author

As before, I am wondering if strict PEP8 compliance is required in python-neo? Most of these line break and remaining indentation complaints won't look very good. Would rather just leave them as is.

@apdavison
Copy link
Member

re PEP 8: I don't think any of us worry about W504 or E126, so for me it's ok not to fix those

@PeterNSteinmetz
Copy link
Contributor Author

Now that the test data is merged in ephy_testing_data I think the tests will pass if restarted.

@samuelgarcia
Copy link
Contributor

I have just rerun travis to see.

@PeterNSteinmetz
Copy link
Contributor Author

It looks like the Travis build is ok. But the errors showing up in the circleci tests have to do with hdf5io . Puzzling as nothing in these commits touches that io. How to fix?

@JuliaSprenger
Copy link
Member

Yes, the h5py checks will be fixed by #893. I will rerun the tests here once #893 is merged.

@PeterNSteinmetz
Copy link
Contributor Author

I guess this should now work if CirclCI is started since PR#893 has been merged.

@PeterNSteinmetz
Copy link
Contributor Author

Going to wait for PR#888 to be merged before trying to rebase or merge this with current master.

@PeterNSteinmetz
Copy link
Contributor Author

Closed and incorporated into new PR.

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