DM-12848: ap_verify unit tests fail on lsst-dev #17
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
kfindeisen
force-pushed
the
tickets/DM-12848
branch
from
65399c4
to
83175ac
Compare
morriscb
approved these changes
Jan 18, 2018
There was a problem hiding this comment.
From what I can see you create a simple dataset for defringing and put that through your timing unittest instead of the dummy ISR. If that's the case things seems clear enough. One suggestion would be to add a line or two of comments in the test setup to to explain the nature of the data your making ie "Create an exposure with some fringe frequency".
| config = lsst.sconsUtils.Configuration( | ||
| __file__, | ||
| hasDoxygenInclude=False, | ||
| hasSwigFiles=False, |
There was a problem hiding this comment.
I don't see anything that changed here. Is this just something weird the commit history?
There was a problem hiding this comment.
That's the "Fix Windows line endings" commit.
|
Thanks, I'll add some comments. |
Dummy task runs are too fast to give good results, but IsrTask is too complex to run on the fly. I've replaced it with FringeTask, which can be run with only minimal setup.
kfindeisen
force-pushed
the
tickets/DM-12848
branch
from
83175ac
to
8b6d490
Compare
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.
This PR rewrites the unit tests for
lsst.ap.verify.measurements.profilingto use a real task execution rather than a dummy one. This gives a running time greater than 0.01 seconds, fixing DM-12848. I also add a missing dependency onip_isrto the package.In a separate commit, I switch the
ups/*.cfgandups/*.tablefiles from Windows to Unix line endings. I have no idea where those came from.