Skip to content
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

tickets/DM-3347 wcsNearlyEqual #34

Merged
merged 2 commits into from Aug 5, 2015
Merged

tickets/DM-3347 wcsNearlyEqual #34

merged 2 commits into from Aug 5, 2015

Conversation

ktlim
Copy link
Contributor

@ktlim ktlim commented Aug 5, 2015

No description provided.

@@ -92,12 +94,16 @@ def assertWcsNearlyEqualOverBBox(testCase, wcs0, wcs1, bbox, maxDiffSky=0.01*afw
diffSky = sky0.angularSeparation(sky1)
if diffSky > measDiffSky[0]:
measDiffSky = (diffSky, fromPixPos)
if doShortCircuit:
break
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These breaks only exit one level. You need to break out of both levels.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good catch. I'll use itertools.product to make the for loop one level deep.

The assertX methods that afw presently adds to lsst_utils TestCase
need only support fail(self, msgStr). Document this, and in one case
make minor code changes to make it true. Also, start the doc strings
with with """! instead of """ to make Doxygen pay attention to
the Doxygen commands.
@r-owen r-owen force-pushed the tickets/DM-3347 branch 2 times, most recently from c89bba6 to 55d8bb9 Compare August 5, 2015 20:13
Both the new wcsNearlyEqualOverBBox and the existing
assertWcsNearlyEqualOverBBox are thin wrappers
around a new private function _compareWcsOverBBox.
Also add a unit test for wcsNearlyEqualOverBBox.
@r-owen r-owen merged commit aac11ed into master Aug 5, 2015
@ktlim ktlim deleted the tickets/DM-3347 branch August 25, 2018 06:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants