Feature/visualize fpr value #42
Conversation
| self.layout.extract.parallel_fpr.median_list_from( | ||
| array=self.data, | ||
| pixels_from_end=min( | ||
| 10, self.layout.smallest_parallel_rows_within_ci_regions |
There was a problem hiding this comment.
Maybe assign 10 to a constant so it's clear what it means rather than just being an arbitrary integer value?
| class ExtractException(Exception): | ||
| pass |
| return [ | ||
| self.region_list[i + 1].x0 - self.region_list[i].x1 | ||
| for i in range(len(self.region_list) - 1) | ||
| ] |
There was a problem hiding this comment.
You can also do
[second.x0 - first.x1 for first, second in zip(self.region_list, self.region_list[1:])]| new_array: aa.Array1D, | ||
| array: aa.Array1D, | ||
| pixels: Optional[Tuple[int, int]] = None, | ||
| pixels_from_end: Optional[int] = None, |
There was a problem hiding this comment.
I guess that decorator I mentioned in the other PR would also be applicable here
| charge injection region. | ||
| """ | ||
| return [ | ||
| self.region_list[i + 1].y0 - self.region_list[i].y1 |
There was a problem hiding this comment.
That zip syntax would also work here
| array.mask[region.slice] for region in self.region_list_from(pixels=pixels) | ||
| array.mask[region.slice] | ||
| for region in self.region_list_from( | ||
| pixels=pixels, pixels_from_end=pixels_from_end |
There was a problem hiding this comment.
That pixels from end to pixels decorator would save so many changes. In particular I would suggest using it at whatever point you first call a function.
Even if you weren't using the decorator doing the conversion then passing pixels rather than two optional arguments to all of these functions would be much cleaner. You've had to make an absolute ton of changes just to support having one of two possible arguments between which there is a simple relationship.
| @property | ||
| def parallel_fpr(self): | ||
| return Extract2DParallelFPR( | ||
| shape_2d=self.shape_2d, | ||
| region_list=self.region_list, | ||
| parallel_overscan=self._parallel_overscan, | ||
| serial_prescan=self._serial_prescan, |
|
|
||
| assert ( | ||
| fpr_list[1].mask | ||
| == np.array([[True, False, False], [False, True, False], [False, False, True]]) |
There was a problem hiding this comment.
I think if you put a comma before the closing bracket this will format over new lines which I find clearer for 2D arrays
The plan was to originally add a simple value of FPR (level of charge injeciton) in certain images for PyAutoCTI.
It turns out, estimating this value required a reworking of the extraction methods which extract the charge injection.