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.

Sign up for GitHub

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

Jump to bottom

adding vtk legacy options to scil_convert_surface script #795

Merged
merged 9 commits into from
Nov 20, 2023
Merged

adding vtk legacy options to scil_convert_surface script #795

merged 9 commits into from
Nov 20, 2023

Conversation

ThoumyreStanislas
Copy link
Contributor

@ThoumyreStanislas ThoumyreStanislas commented Oct 25, 2023 •
edited
Loading

I've modified the scil_convert_surface script so that surfaces created by freesurfer can be viewed in MI-Brain and aligned with T1. An updated version of trimeshpy is required for the script to work, but is not yet available.

@pep8speaks
Copy link

pep8speaks commented Oct 25, 2023 •
edited
Loading

Hello @ThoumyreStanislas, Thank you for updating !

There are currently no PEP 8 issues detected in this Pull Request. Cheers! 🍻

Comment last updated at 2023-11-20 20:17:15 UTC

@ThoumyreStanislas ThoumyreStanislas changed the title correction of scil_convert_surface script so that images are visible in MI-Brain and aligned with t1 WIP adding vtk legacy options to scil_convert_surface script Oct 25, 2023
@arnaudbore arnaudbore self-requested a review October 25, 2023 20:35
@arnaudbore
Copy link
Contributor

Build Failed 💥
https://jenkins.scil.usherbrooke.ca/job/scilpy/job/PR-795/1/

@arnaudbore arnaudbore changed the title WIP adding vtk legacy options to scil_convert_surface script [WIP] adding vtk legacy options to scil_convert_surface script Oct 26, 2023
@arnaudbore
Copy link
Contributor

Build Failed 💥
https://jenkins.scil.usherbrooke.ca/job/scilpy/job/PR-795/2/

@ThoumyreStanislas
Copy link
Contributor Author

I've corrected the code and moved the xform and flip_LPS functions to the scilpy/utils/util.py module.

@arnaudbore
Copy link
Contributor

Build Failed 💥
https://jenkins.scil.usherbrooke.ca/job/scilpy/job/PR-795/4/

@arnaudbore
Copy link
Contributor

Build Failed 💥
https://jenkins.scil.usherbrooke.ca/job/scilpy/job/PR-795/5/

@arnaudbore arnaudbore added the WIP Work In Progress label Nov 2, 2023
@arnaudbore
Copy link
Contributor

Build passed ! Good Job 🍻 !

@arnaudbore arnaudbore requested a review from frheault November 13, 2023 16:45
@ThoumyreStanislas ThoumyreStanislas changed the title [WIP] adding vtk legacy options to scil_convert_surface script adding vtk legacy options to scil_convert_surface script Nov 13, 2023
@arnaudbore
Copy link
Contributor

Build Failed 💥
https://jenkins.scil.usherbrooke.ca/job/scilpy/job/PR-795/7/

@arnaudbore
Copy link
Contributor

Build passed ! Good Job 🍻 !

arnaudbore
arnaudbore reviewed Nov 20, 2023
View reviewed changes
Copy link
Contributor

@arnaudbore arnaudbore left a comment

Choose a reason for hiding this comment

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

LGTM - Need some testing once fixed.

scilpy/surfaces/utils.py Show resolved Hide resolved
scilpy/surfaces/utils.py Show resolved Hide resolved
scilpy/surfaces/utils.py Outdated
return polydata


def extract_xform(filename):
Copy link
Contributor

Choose a reason for hiding this comment

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

At this point you should not deal with files but object.

scilpy/surfaces/utils.py Show resolved Hide resolved
@arnaudbore arnaudbore self-requested a review November 20, 2023 18:57
@arnaudbore
Copy link
Contributor

Build Failed 💥
https://jenkins.scil.usherbrooke.ca/job/scilpy/job/PR-795/10/

@arnaudbore
Copy link
Contributor

Build passed ! Good Job 🍻 !

arnaudbore
arnaudbore approved these changes Nov 20, 2023
View reviewed changes
Copy link
Contributor

@arnaudbore arnaudbore left a comment

Choose a reason for hiding this comment

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

Gooodddd !!!

@arnaudbore arnaudbore merged commit 3227441 into scilus:master Nov 20, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@arnaudbore arnaudbore arnaudbore approved these changes

@frheault frheault Awaiting requested review from frheault

Assignees
No one assigned
Labels
WIP Work In Progress
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@ThoumyreStanislas @pep8speaks @arnaudbore