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

DM-34168: Use only deblended primary sources for TEx metrics #130

Merged
merged 2 commits into from Apr 12, 2022

Conversation

fred3m
Copy link
Contributor

@fred3m fred3m commented Apr 11, 2022

No description provided.

@@ -74,7 +75,7 @@ def test_te1(self):
result = task.run('TE1', {'i': [CalibratedCatalog(catalog), ]})
log.debug('result: ', result)
log.debug('expected: ', expected)
self.assertAlmostEqual(result.measurement, expected, places=10)
np.testing.assert_almost_equal(result.measurement.quantity, expected.quantity, decimal=5)
Copy link
Member

Choose a reason for hiding this comment

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

What's the reason to switch to numpy testing here? I would have thought it made sense for arrays, but that doesn't seem to be the case.

Copy link
Member

Choose a reason for hiding this comment

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

Perhaps, you could add the reason in the body of the commit message? https://developer.lsst.io/work/flow.html?highlight=commit#writing-commit-message-body-content

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sure. FYI it's because assertAlmostEqual doesn't actually work for lsst.verify.measurement.Measurementobjects (since it doesn't have subtraction defined). Unfortunately assertAlmostEqual doesn't work with astropy.quantity.Quantity values either, so I had to both extract the quantity and use numpy's almost equal. 😞

python/lsst/faro/utils/tex.py Outdated Show resolved Hide resolved
python/lsst/faro/utils/tex.py Outdated Show resolved Hide resolved
python/lsst/faro/utils/tex.py Outdated Show resolved Hide resolved
@@ -74,7 +75,7 @@ def test_te1(self):
result = task.run('TE1', {'i': [CalibratedCatalog(catalog), ]})
log.debug('result: ', result)
log.debug('expected: ', expected)
self.assertAlmostEqual(result.measurement, expected, places=10)
np.testing.assert_almost_equal(result.measurement.quantity, expected.quantity, decimal=5)
Copy link
Member

Choose a reason for hiding this comment

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

Perhaps, you could add the reason in the body of the commit message? https://developer.lsst.io/work/flow.html?highlight=commit#writing-commit-message-body-content

fred3m added 2 commits April 11, 2022 19:44
`assertAlmostEqual` doesn't actually work for
`lsst.verify.measurement.Measurement` objects
(since the class doesn't have subtraction defined).

Unfortunately `assertAlmostEqual` doesn't work with
`astropy.quantity.Quantity` values either,
so we are forced to both extract the quantity and
use numpy's almost equal.
@fred3m fred3m merged commit d94a689 into main Apr 12, 2022
@fred3m fred3m deleted the tickets/DM-34168 branch April 12, 2022 00:45
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