DM-12853: Review design questions in ap_verify #19
Conversation
Per discussion on DM-12853, data package management shall be decided by the LSST Project at a later date.
Per discussion on DM-12853, there doesn't seem to be any urgency surrounding the current behavior. It can be deferred until ap_verify's error-handling behavior is better-decided.
python/lsst/ap/verify/dataset.py
Outdated
| try: | ||
| self._dataRootDir = getPackageDir(datasetPackage) | ||
| self._validatePackage() | ||
| except pexExcept.NotFoundError as e: |
There was a problem hiding this comment.
Can _validatePackage throw a pexExcept.NotFoundError? If it can (which I'm dubious about), would it mean that the package hasn't been set up, as the RuntimeError below claims?
There was a problem hiding this comment.
It's getPackageDir that is throwing the NotFoundError if the package is not set up.
There was a problem hiding this comment.
Yes, of course. Hence my comment indicating that there's no point having the try .. except block around _validatePackage...
There was a problem hiding this comment.
I put them in the same try block because self._dataRootDir being initialized is a precondition for _validatePackage, and this way it's easier to see that the precondition will always be satisfied. As noted, _validatePackage will never raise NotFoundError, so the exception handling is unambiguous.
There was a problem hiding this comment.
The usual recommendation is to use an else block with the try. Fluent Python makes an explicit case for doing that (the else only triggers if the try block completed).
kfindeisen
force-pushed
the
tickets/DM-12853
branch
from
be261ba
to
146042c
Compare
This PR updates the
ap_verifydocumentation to comply with the decisions made on DM-12853. Two changes require concurrent updates to source code: renaming--dataIdStringto--idand removing self-setup capability from datasets.The built documentation is available in zip format.