search

kylemath / mne-python

MNE : Magnetoencephalography (MEG) and Electroencephalography (EEG) in Python
http://mne.tools
BSD 3-Clause "New" or "Revised" License
0 stars 1 forks source link

automatic tests are failing on mne-python main repo #17

Open kylemath opened 4 years ago

kylemath commented 4 years ago

Scroll down to the test here: https://github.com/mne-tools/mne-python/pull/7717

for each of them you can click though and find what is making the test fail, the first one is easy so try that first, it will eventually get to a log that indicates a few style issues that are not correct

kuziekj commented 4 years ago

okay, I went through and addressed most of the tests. PR is here, along with more comments:

https://github.com/kylemath/mne-python/pull/16

kuziekj commented 4 years ago

Made some more changes to help with the tests here: https://github.com/kylemath/mne-python/pull/18

It looks like master was updated recently, so we'll need to update importBOXY as well. Looks like it will help with some of the tests.

kylemath commented 4 years ago

ok I merged your PR for the tests, and I rebased to master, so check next you get a chance at how the test do (they take a while)

kuziekj commented 4 years ago

Okay, still getting a few errors. This PR should fix some of them: https://github.com/kylemath/mne-python/pull/19

"mne/tests/test_defaults.py::test_si_units" fails because of the 'csd' channel type (not added by us). Master fixes this by changing a couple values in 'mne\defaults.py', but for some reason those changes weren't incorporated into importBoxy, even though they were added before you updated importBOXY. It looks like those changes are marked for review, so maybe that's why they weren't added to importBOXY?

"mne/io/tests/test_constants.py::test_constants" fails because of the 'fnirs_ph' channel type (and also because of the 'csd' channel type mentioned above). Not sure what to do about this since it seems the devs want to add several new channel types for fnirs data.

"ci/circleci: build_docs artifact" seems to make the html link just fine now, so maybe the next step should be to focus on the tutorial? I can make a separate AC section and phase section, like you suggested earlier, and make any other changes.

kylemath commented 4 years ago

That sounds like a good next step, make the tutorial really straightforward and make sure it renders in the html.well

On Mon., Jun. 22, 2020, 12:18 p.m. kuziekj, notifications@github.com wrote:

Okay, still getting a few errors. This PR should fix some of them: #19 https://github.com/kylemath/mne-python/pull/19

"mne/tests/test_defaults.py::test_si_units" fails because of the 'csd' channel type (not added by us). Master fixes this by changing a couple values in 'mne\defaults.py', but for some reason those changes weren't incorporated into importBoxy, even though they were added before you updated importBOXY. It looks like those changes are marked for review, so maybe that's why they weren't added to importBOXY?

"mne/io/tests/test_constants.py::test_constants" fails because of the 'fnirs_ph' channel type (and also because of the 'csd' channel type mentioned above). Not sure what to do about this since it seems the devs want to add several new channel types for fnirs data.

"ci/circleci: build_docs artifact" seems to make the html link just fine now, so maybe the next step should be to focus on the tutorial? I can make a separate AC section and phase section, like you suggested earlier, and make any other changes.

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/kylemath/mne-python/issues/17#issuecomment-647693632, or unsubscribe https://github.com/notifications/unsubscribe-auth/AA36GFI5ZJQ5YMBYH5S2SV3RX6N7HANCNFSM4OAIAXGQ .

kuziekj commented 4 years ago

Here's a PR for a simplified tutorial: https://github.com/kylemath/mne-python/pull/20

Should also help with another testing issue, where CI was failing because of too many plots. This can probably be further reduced but let me know what you think so far.

kuziekj commented 4 years ago

Here are the html links for read_raw_boxy and the plot_80 tutorial:

https://20750-1301584-gh.circle-artifacts.com/0/dev/generated/mne.io.read_raw_boxy.html?highlight=boxy#mne.io.read_raw_boxy https://20750-1301584-gh.circle-artifacts.com/0/dev/auto_tutorials/preprocessing/plot_80_boxy_processing.html?highlight=boxy

Formatting is off compared to the other tutorials and I'm not sure why, since it's mostly just a modified plot_70. Maybe we missed something though.

CI stills fails, although now it gives a less descriptive error:

embedding documentation hyperlinks for auto_tutorials... [ 98%] plot_eeg_no_mri.html embedding documentation hyperlinks for auto_tutorials... [100%] plot_mne_dspm_source_localization.html

make: *** [Makefile:49: html_dev-pattern] Error 1

Exited with code exit status 2 CircleCI received exit code 2

kylemath commented 4 years ago

you can see in the other documents that were created how the existing tutorial looks to check if it is something specific to the plot_80 file or to your mne build more generally here: https://20750-1301584-gh.circle-artifacts.com/0/dev/auto_tutorials/preprocessing/plot_70_fnirs_processing.html#sphx-glr-auto-tutorials-preprocessing-plot-70-fnirs-processing-py

it seems it is creating the other files correctly, perhaps something about the comment characters that make them show up better in the html?

kuziekj commented 4 years ago

Yeah maybe. When I download the plot_70 tutorial there's only a single comment characters per line, rather than two per line in plot_80. I try changing the comments and hopefully that helps. I'll also incorporate the new channel names from the latest update.

kuziekj commented 4 years ago

Here's a small PR to test the comments in plot_80: https://github.com/kylemath/mne-python/pull/22

Was going to wait for the new channels to add this, but I probably don't need to wait for those changes.

kuziekj commented 4 years ago

well this certainly looks better:

https://20765-1301584-gh.circle-artifacts.com/0/dev/auto_tutorials/preprocessing/plot_80_boxy_processing.html?highlight=boxy https://20765-1301584-gh.circle-artifacts.com/0/dev/generated/mne.io.read_raw_boxy.html?highlight=boxy#mne.io.read_raw_boxy

and yeah, for smaller stuff like this I'll just merge right away. Sorry for the delay.

kylemath commented 4 years ago

this looks amazing, well done Jon, must feel cool to see it as a webpage with all the formatting

On Thu, Jun 25, 2020 at 8:45 AM kuziekj notifications@github.com wrote:

well this certainly looks better:

https://20765-1301584-gh.circle-artifacts.com/0/dev/auto_tutorials/preprocessing/plot_80_boxy_processing.html?highlight=boxy

https://20765-1301584-gh.circle-artifacts.com/0/dev/generated/mne.io.read_raw_boxy.html?highlight=boxy#mne.io.read_raw_boxy

and yeah, for smaller stuff like this I'll just merge right away. Sorry for the delay.

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/kylemath/mne-python/issues/17#issuecomment-649635513, or unsubscribe https://github.com/notifications/unsubscribe-auth/AA36GFOYGMKU7PYR5AE7WRTRYNWKHANCNFSM4OAIAXGQ .

-- Kyle E. Mathewson, Ph.D. Assistant Professor - Department of Psychology, Faculty of Science Director - Attention Perception and Performance Lab Affiliate - Neuroscience and Mental Health Institute, Faculty of Medicine and Dentistry University of Alberta P455 - Biological Sciences Building 11455 Saskatchewan Dr. Edmonton, Alberta, Canada, T6G 2E9 Phone: 1-780-492-2662 Email: kyle.mathewson@ualberta.ca Web: www.kylemathewson.com