nipy / PySurfer

Cortical neuroimaging visualization in Python
https://pysurfer.github.io/
BSD 3-Clause "New" or "Revised" License
240 stars 97 forks source link

WIP: adapt Brain.save_movie() API for imageio #155

Closed christianbrodbeck closed 8 years ago

christianbrodbeck commented 8 years ago

Following up on https://github.com/nipy/PySurfer/pull/154

imageio parameters: http://imageio.readthedocs.io/en/latest/format_ffmpeg.html#parameters-for-saving

Probably the most sense would make to provide access to all of imageio's options. I've saved a couple of movies and imageio seems to have sensible defaults (variable bitrate, hence I removed our fixed bitrate default). We could keep the existing keyword arguments so we don't break code that used positional arguments. Are there better alternatives? I am not sure how to document in case we just include a **kwargs in the signature. Should the documentation just include a link to imageio's documentation?

coveralls commented 8 years ago

Coverage Status

Coverage decreased (-0.03%) to 76.162% when pulling 2a4e85d64b96cac41f0bc515b85f857de8f6c772 on christianbrodbeck:imageio into 89056ab94e7168c3d911a773b468a274e9be597b on nipy:master.

christianbrodbeck commented 8 years ago

Updated. Keeping the specific keywords open would have the big advantage that additional imageio formats would become available, such as *.gif

coveralls commented 8 years ago

Coverage Status

Coverage decreased (-0.03%) to 76.162% when pulling fb5c56d6505edb2318f25951c37934d6a80d0bea on christianbrodbeck:imageio into 89056ab94e7168c3d911a773b468a274e9be597b on nipy:master.

coveralls commented 8 years ago

Coverage Status

Coverage decreased (-0.03%) to 76.162% when pulling fb5c56d6505edb2318f25951c37934d6a80d0bea on christianbrodbeck:imageio into 89056ab94e7168c3d911a773b468a274e9be597b on nipy:master.

coveralls commented 8 years ago

Coverage Status

Coverage decreased (-0.03%) to 76.162% when pulling 756a9a63d3a233f10f52edbe42fe8d5f5d6335c1 on christianbrodbeck:imageio into 89056ab94e7168c3d911a773b468a274e9be597b on nipy:master.

agramfort commented 8 years ago

ready to merge?

christianbrodbeck commented 8 years ago

If you're happy with the documentation

agramfort commented 8 years ago

ok

thx @christianbrodbeck

next time set the PR to MRG so I don't have to ask :)

christianbrodbeck commented 8 years ago

Ok, sorry :)