Make AffineTransformation2D fittable#2277
Closed
embray wants to merge 1 commit into
Closed
Conversation
Merged
Member
Author
|
Now that #2276 is merged this should be doable. |
Member
Author
Member
Author
|
This is still on hold until fitters are redesigned--currently none of the fitters actually support models that have two inputs and two outputs. |
Member
|
@embray - this will likely not be done in time for 1.0, is that right? If so, feel free to remove the 1.0 milestone. |
Member
Author
|
It would be nice--I really wanted to have my fitter refactoring done. But other things have taken too much time. |
Member
|
@nden , is this still relevant? |
Contributor
|
I think AffineTransform will need a new fitting algorithm. Since there's not much in this PR, I'm closing it. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
5 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
#2269 (yet to be merged) introduced a new
AffineTransformation2Dmodel. It was suggested in that PR that it would be good to make instances of this class fittable. Unfortunately this would currently require making it a type ofParametricModel, which is currently broken for models that require multi-dimensional parameter values (such as the transformation matrix used byAffineTransformation2D).Implementing this requires some combination of #2149 and/or #2276 to be merged first. So I decided to create a separate issue for this so that it isn't forgotten.