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

[ENH] added reco80 example #567

Merged
merged 3 commits into from
Nov 19, 2020
Merged

Conversation

36000
Copy link
Collaborator

@36000 36000 commented Oct 21, 2020

@36000 36000 changed the title added reco80 example [WIP] added reco80 example Oct 22, 2020
@36000 36000 force-pushed the reco80_api_example branch 2 times, most recently from 7d3b361 to 1ccdb5b Compare November 10, 2020 22:27
@36000 36000 changed the title [WIP] added reco80 example [ENH] added reco80 example Nov 10, 2020
@36000
Copy link
Collaborator Author

36000 commented Nov 17, 2020

@arokem this is ready for review / merge

@arokem
Copy link
Collaborator

arokem commented Nov 18, 2020

I think this looks fine. Does this take very long to run? Is that why we don't want to run it in the documentation build? I worry about having too many examples that are not exercised by the CI, because they might go out of date, without us noticing. One additional comment is that we can shorten this example a little bit: maybe only run the bundle visualization, and skip the tract profiles and FA visualization. Those are shown in other examples already.

@36000
Copy link
Collaborator Author

36000 commented Nov 18, 2020

@arokem I made those changes. I think it is ready to merge now.
image

@arokem
Copy link
Collaborator

arokem commented Nov 19, 2020

@36000 : I've shortened and simplified the text a bit, focusing on the main differences with other examples (see 74f3f00). If that looks OK to you, I think this is ready to merge.

@36000 36000 merged commit 9a5305f into yeatmanlab:master Nov 19, 2020
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.

None yet

2 participants