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

Fitting result fixes #378

Merged
merged 22 commits into from Sep 2, 2014

Conversation

Projects
None yet
3 participants
@jalabort
Member

jalabort commented May 27, 2014

The intention of this PR is to change some of the functionality of FittingResult.

Its key additions/changes are:

  • Removes previous explicit deepcopy or copy calls from the fitting framework, i.e. from the method fit() (and subsequent calls) of all (Multi) Fitter objects.
  • Removes view...() methods from (Multi) FittingResult (although it remains a Viewable object, i.e. _view() is kept). Instead (Multi) FittingResult now returns images with attached landmarks (see methods fitted_image and iter_image).
  • Removes the property error_type from (Multi) FittingResult. Instead error_type is now a kwarg passed to the method errors(error_type='me'). This simplifies the signature of fit() methods since error_type disappears from its list of kwargs.
  • Removes the fitted flag from (Multi) FittingResult which was unused and probably unnecessary.

Note that, with these changes, (Multi) FittingResult has lost some of its functionalities (only in terms of visualizing the results). These should be brought back to Menpo by #412.

This PR is placed on top of #411

jalabort and others added some commits Aug 21, 2014

Update fittingresult.py
Delete unused properties self._error_type and self._error_text
Merge remote-tracking branch 'upstream/master' into fitting_result_fixes
Conflicts:
	menpo/fitmultilevel/aam/test/aam_builder_test.py
	menpo/fitmultilevel/base.py
Merge remote-tracking branch 'upstream/master' into fitting_result_fixes
Conflicts:
	menpo/fitmultilevel/sdm/test/sdm_test.py

@nontas nontas referenced this pull request Sep 2, 2014

Closed

Training / fitting frameworks refinement #297

23 of 25 tasks complete
@nontas

This comment has been minimized.

Member

nontas commented Sep 2, 2014

I think this is now ready to come in. @jalabort could you please confirm you're happy with my changes?

@jalabort

This comment has been minimized.

Member

jalabort commented Sep 2, 2014

Looks OK to me ;-)


def shapes(self, as_points=False):
if as_points:
return [s.points.copy() for s in self.parameters]
return [deepcopy(s.points) for s in self.parameters]

This comment has been minimized.

@jabooth

jabooth Sep 2, 2014

Member

why have we changed to deepcopy here? Seems .copy() would be preferred...

This comment has been minimized.

@jalabort

jalabort Sep 2, 2014

Member

Good catch James, I think copy should be used here.

Thanks,

Joan

On 2 Sep 2014 16:06, James Booth notifications@github.com wrote:

In menpo/fit/fittingresult.py:

 def shapes(self, as_points=False):
     if as_points:
  •        return [s.points.copy() for s in self.parameters]
    
  •        return [deepcopy(s.points) for s in self.parameters]
    

why have we changed to deepcopy here? Seems .copy() would be preferred...


Reply to this email directly or view it on GitHubhttps://github.com//pull/378/files#r16992441.

jabooth added some commits Sep 2, 2014

jabooth added a commit that referenced this pull request Sep 2, 2014

@jabooth jabooth merged commit ac5d89a into menpo:master Sep 2, 2014

1 check passed

continuous-integration/travis-ci The Travis CI build passed
Details

@jabooth jabooth deleted the jalabort:fitting_result_fixes branch Sep 2, 2014

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