Skip to content

Conversation

@oesteban
Copy link
Member

@oesteban oesteban commented Dec 1, 2017

Make it easier for FMRIPREP to strip svgs from the reports.
HTML files will be used for reportlets (like the summary) that
can be completelly inlined. Otherwise, expect standalone svgs.

Make it easier for FMRIPREP to strip svgs from the reports.
HTML files will be used for reportlets (like the summary) that
can be completelly inlined. Otherwise, expect standalone svgs.
@oesteban oesteban changed the title [WIP,ENH] Generate svgs only [ENH] Generate svgs only Dec 1, 2017
@oesteban oesteban requested a review from chrisgorgo December 1, 2017 17:46
@chrisgorgo
Copy link
Contributor

Could you open WIP PRs for MRIQC and FMRIPREP using this branch so we would see how this is working?

@oesteban
Copy link
Member Author

oesteban commented Dec 1, 2017

@chrisgorgo
Copy link
Contributor

So does this help with large reports that previously were slow to load or crashed browsers?

@oesteban
Copy link
Member Author

oesteban commented Dec 1, 2017

The load time is definitely shorter. I can't claim though that this solves the problem until I see a huge dataset like MyConnectome using this. But that would be an issue of fmriprep, not in niworkflows.

This PR modifies niworkflows:

  • to make it more consistent (ends the dichotomy of registration vs. segmentation reports or svg vs. html respectively),
  • to reduce the codebase and simplify functionality (we can stop caring about html validity, etc)
  • to simplify the tests of report-capable segmentations by leveraging py.tests, as we previously did with tests if registration interfaces.

@chrisgorgo
Copy link
Contributor

Yes it does seem like a major refactor. A bit too much for me to dig through right now, but if it works and does not break MRIQC and FMRIPREP you should be good to go.

@oesteban oesteban merged commit d8872ca into nipreps:master Dec 1, 2017
@oesteban oesteban deleted the enh/generate-svgs branch December 1, 2017 19:56
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.

2 participants