Added tests to compare the length of the extracted Spectrum to the height of the FiberTrace.
Added tests for wavelength calibration using a previously identified reference spectrum as well as using a line list created from the simulated pixel-wavelength map. Check for monotonic wavelength solution within a certain wavelength range.
Added tests to compare the length of the extracted Spectrum to the height of the FiberTrace.
Added tests for wavelength calibration using a previously identified reference spectrum as well as using a line list created from the simulated pixel-wavelength map. Check for monotonic wavelength solution within a certain wavelength range.
Could you please clarify exactly which commits are to be reviewed here? It looks like there's a tickets/PIPE2D-32 branch which contains some commits which haven't been merged to master, but there's also evidence that some version of tickets/PIPE2D-32 did get merged and .. I don't really know where to start. Can you explain what's happened, please?
All commits in tickets/PIPE2D-32 are to be reviewed. I had to merge earlier commits to master to get the tests running for the other tickets again after removing the old testdata and creating drp_stella_data (INFRA-32). In hindsight these changes should probably have been made in tickets/INFRA-32...
Here's a current map of all the branches:
* cac9b28 (HEAD -> master, origin/master, origin/HEAD) Change eigen include to <>, remove specializations for identify from Spectra.cc * 88f7a64 Add obs_pfs and eigen 3.2.5.lsst2 to ups/drp_stella.table * dc89384 fix includes in Math.h | * 0e387d5 (origin/tickets/INFRA-32) Apply comments by reviewer | * 8cb1e8e change output path of 'createDetGeom' to '$OBS_PFS_DIR/pfs/camera/' | * ef2fa55 Move createDetGeom and createRefSpec from .ipynb to .py tasks | | * 9f39051 (origin/tickets/INFRA-36) Add eigen with unsupported to list of required packages | |/ |/| * | 5da73c7 (origin/tickets/PIPE2D-34, origin/tickets/INFRA-31) Merge branch 'master' into tickets/INFRA-31 |\ \ | * | 1d6e19a Change extractAllTracesFromProfile to return a pointer to a SpectrumSet | * | d2b1eee Merge branch 'tickets/PIPE2D-32' | |\ \ * | | | af2a8c8 Adopt final comments of review * | | | 8b2e0e5 change "ImageF(fileName)"+"makeExposure" to "ExposureF(fileName)" * | | | 669b6f3 change concatenating strings with "+" to "os.path.join" * | | | d97ecd2 forgot 2 os.environ.get... * | | | 9e55292 replace "os.eviron.get" with "lsst.utils.getPackageDir" | | | | * dcfae90 (origin/tickets/PIPE2D-32, origin/tickets/INFRA-19) Clean up reduceArc..., add reduceArcTask to tests/testDRP.py | | | | * f71f51b Check RMS of identified lines | | | |/ | | |/| | | * | 28efcf2 Add tests to check for monotonic wavelength solution | | * | 462cef7 Merge branch 'tickets/PIPE2D-26' into tickets/PIPE2D-32 | | |\ \ | | | * | 5a1b8f0 (origin/tickets/PIPE2D-26) Add tests to check number of found FiberTraces and their height | |_|/ / |/| | | * | | | aa634c3 Move test data dir to drp_stella_data * | | | c4ed4b6 Fix env.Prepend to envPrepend |/ / / | * | aeb7d1e Add test to check length of Spectra (should be equal to height of FiberTraces) |/ / * | be176cb Add $PROJECT_DIR/bin to $PATH |/ * 3f1c3c8 re-add needed .ipynb files after purging all big files, add *.ipynb to .gitignore * da0c42f Merge branch 'master' of github.com:Subaru-PFS/drp_stella |\ * | e2d3288 Add drp_stella_data as optional dependency |/ * b214418 Working and relatively clean commit to branch off from for tickets
Looking at that, I've gotta say I'm pretty confuse about what's supposed to go where! For comparison, here's a similar map of afw, which is a pretty complex codebase:
* 52f1810 (HEAD -> master, origin/master, origin/HEAD) Merge branch 'tickets/DM-5293' |\ | * 130674c (origin/tickets/DM-5293, tickets/DM-5293) Fix test to cover both Wcs and TanWcs. |/ * 027f5db Merge pull request #78 from lsst/tickets/DM-5822 |\ | * 38e4d59 (origin/tickets/DM-5822) Fix for optimization dependent fail of convolve.py |/ * f7cf9bf (origin/tickets/DM-1972) Merge branch 'tickets/DM-5462' |\ | * c194e37 (origin/tickets/DM-5462) Minor cleanups to image py files | * bd3949c Add makeRampImage | * 8c9955e Add lsst.afw.geom.testUtils.BoxGrid | * 1e39756 Add NullLinearityType to cameraGeom |/ * a4754ad Merge pull request #77 from lsst/tickets/DM-6090 |\ | * e5f1e81 (origin/tickets/DM-6090) Replace boost::lexical_cast with std equivalent |/ * 0b3f9d4 Merge pull request #76 from lsst/tickets/DM-6089 |\ | * 9efcefc (origin/tickets/DM-6089) Replace boost fixed width integer types with std equivalents |/ * e20427c Merge pull request #75 from lsst/tickets/DM-6325 |\ | * a80364b (origin/tickets/DM-6325) Replace BOOST_STATIC_ASSERT with static_assert |/ * 7bbcb36 Merge pull request #74 from lsst/tickets/DM-6094
I hope you'll agree that's much clearer and easier to figure out which commits go with which tickets! Please, for my sanity if for no other reason, try to keep history easy to follow. 
I think the work relevant for this ticket is on the following commits:
- aeb7d1e
- 28efcf2
- f71f51b
- dcfae90
While, in addition, 5a1b8f0 needs reviewing as part of PIPE2D-26.
Please check and make sure you agree with this list before I proceed.
Oh wow, that does look pretty confusing. Sorry about that. I promise to keep that in mind in the future. I agree with the relevant commits for this ticket, plus that 5a1b8f0 needs reviewing as part of PIPE2D-26.
Looking at the afw commits it seems that all tickets had been reviewed and merged with master before a new ticket was started. I doubt though that we can do this all the time... To my defense, purging the large files from the drp_stella repo did brake the tests so I thought it was a good idea to merge/rebase some tickets. I believe that things like this won't happen again...
Andreas will clean up the branches, moving the four commits above to a clean branch rebased against master, then I'll take another look at the review.
Changes accidentally already merged into master as part of a different ticket.