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

Allow passing exporter kwargs - allow path only exporters #737

Merged
merged 4 commits into from Sep 11, 2016

Conversation

Projects
None yet
1 participant
@patricksnape
Contributor

patricksnape commented Sep 3, 2016

This makes two changes - one is that we now support path only
exporters (via a private method). This has been dogfooded inside
and was really intended to be used with Menpo3D since the VTK
exporters can't write directly buffers. Actually, we already
had this paradigm with videos so this just formalises that.

It also allows passing extra kwargs to exporters in the same
manner that videos already can.

Allow passing exporter kwargs - allow path only exporters
This makes two changes - one is that we now support path only
exporters (via a private method). This has been dogfooded inside
and was really intended to be used with Menpo3D since the VTK
exporters can't write directly buffers. Actually, we already
had this paradigm with videos so this just formalises that.

It also allows passing extra kwargs to exporters in the same
manner that videos already can.

patricksnape added a commit to patricksnape/menpo3d that referenced this pull request Sep 3, 2016

Allow path only exporters
This requires menpo/menpo#737
to be merged.
@patricksnape

This comment has been minimized.

Contributor

patricksnape commented Sep 3, 2016

@jabooth review would be very much appreciated pal.

@patricksnape

This comment has been minimized.

Contributor

patricksnape commented Sep 6, 2016

@jabooth Ping 😄

patricksnape added some commits Sep 6, 2016

HOTFIX: exporting landmark files Python 3
JSON dump doesn't support writing to 'wb' opened files - so
we create our own overridden exporter that encodes the strings
as UTF8
@patricksnape

This comment has been minimized.

Contributor

patricksnape commented Sep 7, 2016

@jabooth

import menpo.io as mio

i = mio.import_builtin_asset.breakingbad_jpg()

mio.export_video([i, i, i], '/tmp/test.gif', fps=1, overwrite=True)
mio.import_image('/tmp/test.gif')[0]
---------------------------------------------------------------------------
AttributeError                            Traceback (most recent call last)
<ipython-input-2-dc0334e19a67> in <module>()
----> 1 mio.import_image('/tmp/test.gif')[0].view()

/Users/pts08/gits/menpo/menpo/io/input/base.py in import_image(filepath, landmark_resolver, normalize, normalise)
    259                    landmark_resolver=landmark_resolver,
    260                    landmark_attach_func=_import_object_attach_landmarks,
--> 261                    importer_kwargs=kwargs)
    262 
    263 

/Users/pts08/gits/menpo/menpo/io/input/base.py in _import(filepath, extensions_map, landmark_resolver, landmark_ext_map, landmark_attach_func, asset, importer_kwargs)
    875         if landmark_attach_func is not None:
    876             landmark_attach_func(built_objects, landmark_resolver,
--> 877                                  landmark_ext_map=landmark_ext_map)
    878 
    879     if len(built_objects) == 1:

/Users/pts08/gits/menpo/menpo/io/input/base.py in _import_object_attach_landmarks(built_objects, landmark_resolver, landmark_ext_map)
    785             for group_name, lm_path in lm_paths.items():
    786                 lms = _import(lm_path, landmark_ext_map, asset=x)
--> 787                 if x.n_dims == lms.n_dims:
    788                     x.landmarks[group_name] = lms
    789 

AttributeError: 'LazyList' object has no attribute 'n_dims'

This is a logic bug that is difficult to fix. The image importer is using the default landmark_resolver which expects to be able to attach images to each image recovered from the importer. However, because of the now lazy importing of images this fails as the resolver attempts to attach the landmarks directly to the list. In this case the user should really be using a lazy resolver such as the one used for videos. How should we handle this so that this problem is either avoided or more transparent to the user?

@patricksnape

This comment has been minimized.

Contributor

patricksnape commented Sep 10, 2016

To fix the gif bug we are suggesting the gifs can only imported and exported using the video mode. In the case where multiple images are returned from a gif we throw an error and tell the user to use the video importer.

@patricksnape

This comment has been minimized.

Contributor

patricksnape commented Sep 11, 2016

Merging this - the gif behaviour is breaking so will go in another PR.

@patricksnape patricksnape merged commit 33e77d8 into menpo:master Sep 11, 2016

2 checks passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
macOS MenpoBot Jenkins build passed
Details

@patricksnape patricksnape deleted the patricksnape:export_kwargs branch Sep 11, 2016

@patricksnape patricksnape referenced this pull request Sep 11, 2016

Open

GIF importing #740

patricksnape added a commit to menpo/menpo3d that referenced this pull request Sep 21, 2016

Support for PLY format (#31)
* Initial import working for nontextured meshes

* Export working

* Clean up debug messages

* Allow path only exporters

This requires menpo/menpo#737
to be merged.

* Fix export tests

Mild changes to ply exporter
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment