Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

removed requirement for curv.*h files to create Brain object #11704

Merged
merged 6 commits into from
Jun 14, 2023

Conversation

Aaronearlerichardson
Copy link
Contributor

Reference issue

Fixes #11703

What does this implement/fix?

Makes curv.*h files optional for creating Brain objects

@welcome
Copy link

welcome bot commented May 23, 2023

Hello! 👋 Thanks for opening your first pull request here! ❤️ We will try to get back to you soon. 🚴

@larsoner
Copy link
Member

Makes sense to me! Can you add this change to doc/changes/latest.inc using :newcontrib: and add your name to doc/changes/names.inc?

@hoechenberger
Copy link
Member

What is the status of this? 🙃

@Aaronearlerichardson
Copy link
Contributor Author

I added the requested changes, and am waiting for someone to merge it I think?

@hoechenberger
Copy link
Member

@larsoner Could you please take a look? 😄

@larsoner
Copy link
Member

larsoner commented Jun 8, 2023

Nice! I think the one last thing missing now is a regression test. I would add it to mne/viz/_brain/tests/ somewhere. We might already have tests that make a tmp_path copy of a subject -- you could modify one to remove the lh.curv for example after it's shutil.copytreed to the temp dir, and that should cover the new lines (the curv should be missing when the brain is plotted).

If it's not clear how to do it let me know and I can push a quick commit

@larsoner larsoner enabled auto-merge (squash) June 14, 2023 16:27
@larsoner
Copy link
Member

Pushed a tiny test and marked for merge when green, thanks in advance Aaronearlerichardson !

@larsoner larsoner merged commit bb1d547 into mne-tools:main Jun 14, 2023
@welcome
Copy link

welcome bot commented Jun 14, 2023

🎉 Congrats on merging your first pull request! 🥳 Looking forward to seeing more from you in the future! 💪

larsoner added a commit to larsoner/mne-python that referenced this pull request Jun 23, 2023
* upstream/main: (24 commits)
  Allow int-like as ID of make_fixed_length_events (mne-tools#11748)
  Easycap-M43 montage (mne-tools#11744)
  ENH: Create a Calibrations class for eyetracking data (mne-tools#11719)
  Fix alphabetical order in overview/people.rst, fix sphinx formatting in docstrings and set verbose to keyword-only (mne-tools#11745)
  Add Mathieu Scheltienne to MNE-Python Steering Council (mne-tools#11741)
  removed requirement for curv.*h files to create Brain object (mne-tools#11704)
  [BUG] Fix mne.viz.Brain.add_volume_labels matrix ordering bug (mne-tools#11730)
  Fix installer links (mne-tools#11729)
  MAINT: Update for PyVista deprecation (mne-tools#11727)
  MAINT: Update roadmap (mne-tools#11724)
  MAINT: Update download link [skip azp] [skip cirrus] [skip actions]
  fix case for chpi_info[1] == None (mne-tools#11714)
  Add cmap argument for mne.viz.utils.plot_sensors (mne-tools#11720)
  BUG: Fix one more PySide6 bug (mne-tools#11723)
  MAINT: Fix PySide6 and PyVista compat (mne-tools#11721)
  MRG: If _check_fname() cannot find a file, display the path in quotation marks to help spot accidental trailing spaces (mne-tools#11718)
  Add "array-like" to `_validate_type()` (mne-tools#11713)
  MAINT: Avoid problematic PySide6 (mne-tools#11715)
  Fix installer links (mne-tools#11709)
  Updating change log after PR mne-tools#11575 (mne-tools#11707)
  ...
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.

Creation of Brain object should not require curv.*h file
3 participants