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 exporting of 3D landmarks. #615

Merged
merged 3 commits into from Jul 27, 2015

Conversation

Projects
None yet
4 participants
@mmcauliffe
Contributor

mmcauliffe commented Jul 24, 2015

Using export_landmark_file from menpo3d.io currently only outputs points with two dimensions even for meshes.

I only updated the LJSON exporter, since it looks like the PTS one does some swapping of axes, so I don't know where the z axis would fall in there.

@menpobot

This comment has been minimized.

menpobot commented Jul 24, 2015

Can one of the admins verify this patch?

@patricksnape

This comment has been minimized.

Contributor

patricksnape commented Jul 26, 2015

Hi Michael!

Thanks a lot for this - from the face of it, this looks like a pretty poor oversight by us :( Also, thanks for fixing it! @jabooth Do you have a mesh handy to try this out?

@jabooth

This comment has been minimized.

Member

jabooth commented Jul 26, 2015

Thanks @mmcauliffe!
@menpobot, test this please.

@patricksnape could use landmarker data for test?
https://github.com/menpo/landmarker.io/blob/master/api/v2/landmarks/james/ibug68.json

Load, save, verify unchanged?

@jabooth

This comment has been minimized.

Member

jabooth commented Jul 26, 2015

ok to test

@patricksnape

This comment has been minimized.

Contributor

patricksnape commented Jul 27, 2015

All I want is a test for this, so we know if some idiot (me) breaks this again 😄

@patricksnape

This comment has been minimized.

Contributor

patricksnape commented Jul 27, 2015

@mmcauliffe Can you add this test into the io_export_test.py somewhere please?

@patch('menpo.io.output.landmark.json.dump')
@patch('menpo.io.output.base.Path.exists')
@patch('{}.open'.format(__name__), create=True)
def test_export_landmark_ljson_3d(mock_open, exists, json_dump):
    exists.return_value = False
    fake_path = '/fake/fake3d.ljson'
    test3d_lg = test_lg.copy()
    fake_z_points = np.random.random(test3d_lg.lms.points.shape[0])
    test3d_lg.lms.points = np.concatenate([
        test3d_lg.lms.points, fake_z_points[..., None]], axis=-1)

    with open(fake_path) as f:
        type(f).name = PropertyMock(return_value=fake_path)
        mio.export_landmark_file(test3d_lg, f, extension='ljson')

    json_dump.assert_called_once()
    json_points = np.array(json_dump.call_args[0][0]['landmarks']['points'])
    assert_allclose(json_points[:, -1], fake_z_points)

mmcauliffe added some commits Jul 27, 2015

@mmcauliffe

This comment has been minimized.

Contributor

mmcauliffe commented Jul 27, 2015

I added the test in! I also just had to add an import for assert_allclose, but it's all good now. Let me know if you need me to add anything else!

@jabooth

This comment has been minimized.

Member

jabooth commented Jul 27, 2015

Looks great! :) Thanks Michael, really appreciate it.
+1

jabooth added a commit that referenced this pull request Jul 27, 2015

Merge pull request #615 from mmcauliffe/master
Allow exporting of 3D landmarks.

@jabooth jabooth merged commit 72db971 into menpo:master Jul 27, 2015

4 checks passed

OS X MenpoBot Jenkins build passed No test results found.
Details
clahub All contributors have signed the Contributor License Agreement.
Details
continuous-integration/appveyor AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment