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

MRG: Add support for reading and writing sufaces to .obj #7824

Merged
merged 14 commits into from
Jun 13, 2020

Conversation

wmvanvliet
Copy link
Contributor

@wmvanvliet wmvanvliet commented May 25, 2020

Add support for exporting FreeSurfer surfaces to .obj files so you can import them in blender.
blender

See also: https://github.com/ezemikulan/blender_freesurfer

Todo:

  • Implement writing .obj files
  • Implement reading .obj files
  • Basic unit tests
  • Finish unit tests for full coverage
  • Add tutorial on how to use this to fix Surface inner skull is not completely inside surface outer skull errors
  • Update what's new

mne/surface.py Outdated
for v in coords:
fid.write('v {} {} {}\n'.format(*v))
for f in faces:
fid.write('f {} {} {}\n'.format(*(f + 1)))
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@wmvanvliet we are allowed to use f-strings now because we require 3.6+

@@ -244,4 +252,4 @@ def test_normal_orth():
assert_allclose(ori[2], nn, atol=1e-12)


run_tests_if_main()
#run_tests_if_main()
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

remove

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

BTW now that we have pytest and it's (much) easier to use, I find these not so helpful. I'd be okay with getting rid of them in the codebase...

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Alternatively, is there a good way to run a single test inside a Jupyter console? (I usually need the postmortem debugger)

@larsoner
Copy link
Member

Alternatively, is there a good way to run a single test inside a Jupyter console? (I usually need the postmortem debugger)

Personally I just do:

pytest mne/tests/test_whatever.py -k test_something --tb=short --pdb

this drops to a (pdb) prompt where you can even do interactive and then have a standard python terminal (with matplotlib interactive, etc.). Not sure if there is a way to get this to be an ipython rather than python terminal, maybe with something like https://cprohm.de/article/ipytest-running-pytest-in-ipython-notebooks.html/ ?

@wmvanvliet
Copy link
Contributor Author

Sphinx-gallery gallery_conf["plot_gallery"] was False, so no examples were executed.

Does that mean that CircleCI is complaining that no examples needed to be build or something?

@larsoner
Copy link
Member

No, it's a warning about optipng (warnings are treated as errors) that you can safely ignore. It's because your branch is out of date and CircleCI doesn't / can't merge the yaml before executing

Copy link
Member

@larsoner larsoner left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks like it just needs latest.inc update then +1 for merge from me

@wmvanvliet
Copy link
Contributor Author

I'm still working on the "fix my BEM with Blender" tutorial, which I feel should be part of this PR.

@wmvanvliet
Copy link
Contributor Author

wmvanvliet commented Jun 12, 2020

Can anyone help me getting the static images (blender screenshots) properly embedded the tutorial text? @larsoner @drammock

@larsoner
Copy link
Member

I can push a commit to fix the warnings and add a thumbnail (hopefully)

@larsoner
Copy link
Member

(FYI testing locally with PATTERN=in_blender make html_dev-pattern allows for fairly quick iteration to fix bugs)

Grr... something is wrong with the SG static thumb embedding. I'll probably need to fix that at the SG end, but the syntax here is correct so shouldn't be a blocker for merge

@larsoner
Copy link
Member

... turns out I just can't spell

mne/surface.py Outdated Show resolved Hide resolved
@wmvanvliet wmvanvliet changed the title WIP: Add support for reading and writing sufaces to .obj MRG: Add support for reading and writing sufaces to .obj Jun 12, 2020
@wmvanvliet
Copy link
Contributor Author

When CI comes back green, this is ready to merge from my end.

@larsoner
Copy link
Member

Green other than timeouts that are not your fault that we can ignore

@larsoner
Copy link
Member

The files are actually kind of big (500 / 700 kB), maybe some suitable JPG compression could be used?

Copy link
Member

@larsoner larsoner left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Other than the latest.inc tweak LGTM

@ezemikulan do you want to have a look?

doc/changes/latest.inc Outdated Show resolved Hide resolved
@wmvanvliet
Copy link
Contributor Author

Image filesizes are around 100kb now

Copy link
Member

@agramfort agramfort left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

not sure why CIs complain but it looks good to me

@wmvanvliet
Copy link
Contributor Author

CI's were timing out

@wmvanvliet wmvanvliet merged commit 78bc2a4 into mne-tools:master Jun 13, 2020
@wmvanvliet wmvanvliet deleted the surf_to_obj branch June 13, 2020 17:13
@ezemikulan
Copy link
Contributor

Hi, sorry for the late reply, looks great to me!

@wmvanvliet
Copy link
Contributor Author

Hi @ezemikulan, thanks! If you can state you full name I can add you properly instead of just your username.

@ezemikulan
Copy link
Contributor

It is Ezequiel Mikulan

Thanks again

larsoner added a commit to larsoner/mne-python that referenced this pull request Jun 17, 2020
* upstream/master: (24 commits)
  WIP: Fix Travis (mne-tools#7906)
  WIP: Prototype of notebook viz (screencast) (mne-tools#7758)
  MRG, FIX: Speed up I/O tests, mark some slow (mne-tools#7904)
  Proper attribution for Blender tutorial (mne-tools#7900)
  MAINT: Check usage [ci skip] (mne-tools#7902)
  Allow find_bad_channels_maxwell() to return scores (mne-tools#7845)
  Warn if NIRx directory structure has been modified from original format (mne-tools#7898)
  Pin pvyista to 0.24.3 (mne-tools#7899)
  MRG: Add support for reading and writing sufaces to .obj (mne-tools#7824)
  Fix _auto_topomap_coords docstring. (mne-tools#7895)
  MRG, FIX: Ensure Info H5-writeable (mne-tools#7887)
  Website contents (mne-tools#7889)
  MRG, ENH: Add mri_resolution="sparse" (mne-tools#7888)
  MRG, ENH: Allow disabling FXAA (mne-tools#7877)
  remove "and and" [ci skip] (mne-tools#7882)
  fix evoked nave → inverse guidance (mne-tools#7881)
  ENH: Better error messages (mne-tools#7879)
  FIX : EDF+ Annotation Timestamps missing sub-second accuracy (mne-tools#7875)
  FIX: Fix get_channel_types (mne-tools#7878)
  MRG, BUG: Fix combine evokeds (mne-tools#7869)
  ...
larsoner added a commit to larsoner/mne-python that referenced this pull request Jun 25, 2020
* upstream/master: (23 commits)
  MAINT: Add mne.surface to docstring tests (mne-tools#7930)
  MRG: Add smoothing controller to TimeViewer for the notebook backend (mne-tools#7928)
  MRG: TimeViewer matplotlib figure color (mne-tools#7925)
  fix typos (mne-tools#7924)
  MRG, ENH: Add method to project onto max power ori (mne-tools#7883)
  WIP: Warn if untested NIRX device (mne-tools#7905)
  MRG, BUG: Fix bug with volume morph and subject_to!="fsaverage" (mne-tools#7896)
  MRG, MAINT: Clean up use of bool, float, int (mne-tools#7917)
  ENH: Better error message for incompatible Evoked objects (mne-tools#7910)
  try to fix nullcontext (mne-tools#7908)
  WIP: Fix Travis (mne-tools#7906)
  WIP: Prototype of notebook viz (screencast) (mne-tools#7758)
  MRG, FIX: Speed up I/O tests, mark some slow (mne-tools#7904)
  Proper attribution for Blender tutorial (mne-tools#7900)
  MAINT: Check usage [ci skip] (mne-tools#7902)
  Allow find_bad_channels_maxwell() to return scores (mne-tools#7845)
  Warn if NIRx directory structure has been modified from original format (mne-tools#7898)
  Pin pvyista to 0.24.3 (mne-tools#7899)
  MRG: Add support for reading and writing sufaces to .obj (mne-tools#7824)
  Fix _auto_topomap_coords docstring. (mne-tools#7895)
  ...
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