[PIPE2D-26] Write tests for FiberTraceSet generation Created: 03/Sep/14  Updated: 29/Jul/16  Resolved: 29/Jul/16

Status: Done
Project: DRP 2-D Pipeline
Component/s: None
Affects Version/s: None
Fix Version/s: None

Type: Story Priority: Major
Reporter: rhl Assignee: aritter
Resolution: Done Votes: 0
Labels: None
Remaining Estimate: Not Specified
Time Spent: Not Specified
Original Estimate: Not Specified

Issue Links:
Blocks
is blocked by INFRA-31 Create repo for unittest data files Done
Story Points: 6
Epic Link: Initial Fiber Trace pipeline
Sprint: 2014-9, 2014-12, 2014-13, 2014-14, 2014-15
Reviewers: swinbank

 Description   

Write tests for FiberTraceSet using LSST-style wrapping of python's unittest



 Comments   
Comment by Anonymous [ 05/Feb/15 ]

@rhl: What exactly do you mean by LSST-style wrapping of python's unittest? I have written python unittests for FiberTraces, Spectra, and PSF. Also, this issue is pretty much the same is INFRA-20? How should I name git branches for issues like INFRA-20 and PIPE2D-20? Standard I think is tickets/0020 for both of them...

Comment by aritter [ 29/Jun/16 ]

Added tests to check number and length of FiberTraces as now we have kind of realistic simulated images which can stick around and won't be redundant every time we get new simulations

Comment by swinbank [ 18/Jul/16 ]

As far as I can see, the total amount of test code added as part of this ticket is very small. However, it's a delta on top of a large amount of existing code in tests/FiberTraces.py. The issue description isn't really specific enough for me to understand whether the existing code + the new changes fulfil the goals. These changes do check that we extract the correct number of fibre traces from some predefined test data, and then verifies that their size lies within acceptable ranges. That seems like a good addition.

I have only minor comments on the new code:

  • Please try not to add extra, unnecessary white space. (Line 44 of tests/FiberTraces.py reads " \n", which is a bit useless.)
  • It would be good to specify why these particular values for the constants nFiberTraces, minLength and maxLength were chosen. The answer is presumably just that these correspond to the particular test data available in the repository, but, if so, say so.
  • In general, the code in this file consists of a few very, very long methods. The new code just makes one of them longer. This is a really bad way to structure tests: much better to break them up into fine-grained chunks. I don't think restructuring the whole file is in scope for this ticket, but consider whether these new lines could be broken out into a separate method.
Comment by aritter [ 29/Jul/16 ]

Added comment about nFiberTraces, minLength, and maxLength as advised by the reviewer. Merged with master.

Generated at Sat Feb 10 15:47:29 JST 2024 using Jira 8.3.4#803005-sha1:1f96e09b3c60279a408a2ae47be3c745f571388b.