DM-15045: Activate convolved measurements on undeblended sources #10
Conversation
| flag = schema.addField(name + "_kron_flag", type="Flag", | ||
| doc="convolved Kron flux failed: seeing %f" % (seeing,)), | ||
| flag=schema.addField(name + "_kron_flag", type="Flag", | ||
| doc="convolved Kron flux failed: seeing %f" % (seeing,)), |
There was a problem hiding this comment.
Perhaps for next time, schema.join(name, 'kron', 'flag') would be the preferred way to concatenate schema name components these days. We haven't made a push to use it across the board, though, so I won't insist on doing it on a high-priority fix ticket like this one.
| @@ -478,7 +486,7 @@ def measureAperture(self, measRecord, exposure, aperturePhot): | |||
| """ | |||
| try: | |||
| aperturePhot.measure(measRecord, exposure) | |||
| except: | |||
| except: # noqa E722: want to catch any and all errors | |||
There was a problem hiding this comment.
Still better to do except Exception, so you don't block e.g. KeyboardInterrupt, I think.
| "because the column names are derived from the configuration rather than\n" | ||
| "being static. Sometimes you want to turn this off, e.g., when you will\n" | ||
| "use aperture corrections derived from somewhere else through the 'proxy'\n" | ||
| "mechanism.") |
There was a problem hiding this comment.
I didn't know what "instantiation time" meant, exactly, when first reading this. Maybe better to say "when the Plugin is instantiated".
tests/test_convolved.py
Outdated
| self.assertGreater(source.get(prefix + kronName + "_fluxSigma"), 0) | ||
| self.assertFalse(source.get(prefix + kronName + "_flag")) | ||
|
|
||
| # Aperture measurements suceeded and match expectation |
Would get the following exception when activating convolved measurements of undeblended sources: lsst::pex::exceptions::InvalidParameterError: 'Field with name 'ext_convolved_ConvolvedFlux_0_deconv' already present in schema.' This was due to assuming the algorithm name was set in stone. Extended tests to catch this and test the undeblended measurements as well.
from the build system.
PaulPrice
force-pushed
the
tickets/DM-15045
branch
2 times, most recently
from
d5fb27c
to
0f5f9f9
Compare
setup.cfg
Outdated
| exclude = | ||
| __init__.py, | ||
| doc/conf.py, | ||
| python/lsst/afw/cameraGeom/cameraGeomLib.py, |
There was a problem hiding this comment.
You probably want to clean up these exclusions.
tests/test_convolved.py
Outdated
| @@ -243,7 +255,7 @@ def setup_module(module, backend="virtualDevice"): | |||
| lsst.utils.tests.init() | |||
| try: | |||
| afwDisplay.setDefaultBackend(backend) | |||
| except: | |||
| except: # noqa E722: want to catch any and all errors | |||
There was a problem hiding this comment.
Please catch BaseException. It is no longer standard practice in python3 to have a bare except. The point is that the idiom now is for you to have to think whether Exception or BaseException should be caught and make that decision explicit. BaseException should only be used if you want to ignore typos in your code, sys.exit, and ctrl-C and the like.
PaulPrice
force-pushed
the
tickets/DM-15045
branch
from
0f5f9f9
to
b558bc0
Compare
Extraneous spaces, bare 'except'.
The aperture correction registration has been done when the plugin is instantiated because the column names are derived from the configuration rather than being static. Sometimes you want to turn this off, e.g., when you will use aperture corrections derived from somewhere else through the 'proxy' mechanism.
PaulPrice
force-pushed
the
tickets/DM-15045
branch
from
b558bc0
to
5ce7b43
Compare
No description provided.