PiecewiseAffineTransform.estimate return should reflect underlying transforms #6211
Conversation
This file contains 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
grlee77
approved these changes
Jan 26, 2022
There was a problem hiding this comment.
This fix looks sensible to me. Thanks @chrisroat (will merge in a couple days if there are no further comments)
grlee77
added
🩹 type: Bug fix
rfezzani
reviewed
Jan 27, 2022
There was a problem hiding this comment.
Thank you @chrisroat, I simply suggest to rename the ok variable to success to match the docstring, otherwise it LGTM 😉
Co-authored-by: Riadh Fezzani <rfezzani@gmail.com>
|
Thank you @chrisroat! |
|
@meeseeksdev backport to v0.19.x |
meeseeksmachine
pushed a commit
to meeseeksmachine/scikit-image
that referenced
this pull request
Feb 14, 2022
…rn should reflect underlying transforms
grlee77
added a commit
that referenced
this pull request
Feb 15, 2022
…1-on-v0.19.x Backport PR #6211 on branch v0.19.x (PiecewiseAffineTransform.estimate return should reflect underlying transforms)
tibuch
pushed a commit
to fmi-faim/scikit-image
that referenced
this pull request
Mar 14, 2022
…ansforms (scikit-image#6211) * PiecewiseAffineTransform.estimate return should reflect underlying transforms Co-authored-by: Riadh Fezzani <rfezzani@gmail.com>
rapids-bot bot
pushed a commit
to rapidsai/cucim
that referenced
this pull request
Mar 25, 2022
) Closes #199 Corresponds to: scikit-image/scikit-image#6207 scikit-image/scikit-image#6211 scikit-image/scikit-image#6214 Additionally, the affines contained within the `PiecewiseAffineTransform` will have parameters in CuPy arrays when the inputs to `estimate` are CuPy arrays. This is one to make it consistent with the other transform classes. @chrisroat, can you confirm if this fixes the issue for you? Authors: - Gregory Lee (https://github.com/grlee77) Approvers: - https://github.com/jakirkham URL: #208
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Description
Prior to this change, PiecewiseAffineTransform.estimate always returns True -- even when one of the underlying transforms is ill-conditioned. This change causes a return value of True only when all the transforms are well-conditioned.
Closes #6206
Checklist
For reviewers
later.
__init__.py.doc/release/release_dev.rst.example, to backport to v0.19.x after merging, add the following in a PR
comment:
@meeseeksdev backport to v0.19.xrun-benchmarklabel. To rerun, the labelcan be removed and then added again. The benchmark output can be checked in
the "Actions" tab.