jonescompneurolab / hnn-core

Simulation and optimization of neural circuits for MEG/EEG source estimates
https://jonescompneurolab.github.io/hnn-core/
BSD 3-Clause "New" or "Revised" License
55 stars 52 forks source link

[MRG] DOC: add 'About' section to README.rst #677

Closed rythorpe closed 1 year ago

rythorpe commented 1 year ago

fixes #676

codecov-commenter commented 1 year ago

Codecov Report

All modified lines are covered by tests :white_check_mark:

Comparison is base (a8df9a0) 91.42% compared to head (c64f9b9) 91.42%.

:exclamation: Current head c64f9b9 differs from pull request most recent head baaa882. Consider uploading reports for the commit baaa882 to get more accurate results

:exclamation: Your organization needs to install the Codecov GitHub app to enable full functionality.

Additional details and impacted files ```diff @@ Coverage Diff @@ ## master #677 +/- ## ======================================= Coverage 91.42% 91.42% ======================================= Files 25 25 Lines 4604 4604 ======================================= Hits 4209 4209 Misses 395 395 ```

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

rythorpe commented 1 year ago

We're now getting the same matplotlib error for the ubuntu build as with the windows build: FAILED hnn_core/tests/test_viz.py::test_network_visualization - AttributeError: 'FigureCanvasAgg' object has no attribute 'button_press_event'. Any idea how to fix this @jasmainak?

jasmainak commented 1 year ago

looks like they've changed their API slightly. See this example:

https://matplotlib.org/stable/gallery/event_handling/coords_demo.html

we need to update our test. If it's used outside of tests, we may need some if/else how MNE does it:

https://github.com/mne-tools/mne-python/blob/079c868240a898204bf82b2f1bf0e04cdee75da1/mne/viz/utils.py#L864

rythorpe commented 1 year ago

Hmm, this is looking a bit more involved than I was expecting. I'll need to orient myself with the GUI code more before proceeding - if anyone wants to give it a shot, go for it. Otherwise, I'll try to tackle it next week.

rythorpe commented 1 year ago

I moved all the other stuff about fixing the recent dependency breakage to #678 so that we can move forward with this PR despite the tests not passing.

Feel free to merge whenever you're ready @ntolley!

ntolley commented 1 year ago

Looks like I don't have the right permissions to merge! @jasmainak can you merge when the tests are failing? Or is this hard-coded in our security settings

rythorpe commented 1 year ago

Any update here @jasmainak?

jasmainak commented 1 year ago

I over-rode the failing test and merged. Thanks @rythorpe for the PR ! Let's try to fix the failing test as soon as we can