DM-39131: Avoid setting infinite limits in scatterPlot #103
Conversation
| # statistics | ||
| for name in self._stats: | ||
| base.append((f"{{band}}_highSN{defaults.suffix_stat}_{name}", Scalar)) | ||
| base.append((f"{{band}}_lowSN{defaults.suffix_stat}_{name}", Scalar)) | ||
| base.append(("lowSnThreshold", Scalar)) |
There was a problem hiding this comment.
Isn't it? So much so that I added similar loop roll-ups for the marginal histograms. Also, it now loops over self.plotTypes rather than looping over all possible plot types and checking if they're in self.plotTypes, so you can set the order of plotting however you like.
| colormaps_datatype = { | ||
| "stars": newBlues, | ||
| "galaxies": newReds, | ||
| } |
There was a problem hiding this comment.
There should be colour maps for the other types, even if they are just set to None for now and fixed later.
There was a problem hiding this comment.
I added the colormap as an attribute but couldn't find colormaps for unknown or all/any. There is a divergent one with three colors in skyPlot.py.
| if "any" in self.plotTypes: # type: ignore | ||
| totalX.append(cast(Vector, data["x"])) | ||
|
|
||
| # This filters out nans, but that probably should have happened earlier |
There was a problem hiding this comment.
This should have happened earlier, I don't like taking NaNs out of x and not their corresponding y.
There was a problem hiding this comment.
It's fine because it's just for histograms. In fact all of the plotting calls will filter out non-finite points, so the [x for x in totalX if x == x] lines were unnecessary. The only issues come with trying to set limits without filtering non-finite points (not just nan ones).
I do agree that filtering should happen earlier. Probably most plots should select S/N > 3 or 5, like the scatterplot unit test does.
| @@ -1,33 +1,38 @@ | |||
| 2.165581377360398818e+01 2.537420191914861789e+00 | |||
There was a problem hiding this comment.
These tests are really annoying but I think I will make a future ticket to sort out testing in atools in general.
| print(f"{xLims=}") | ||
| xLims[0] = min(xLims[0], xMin) | ||
| xLims[1] = max(xLims[1], xMax) | ||
| print(f"{xLims=}") |
There was a problem hiding this comment.
Do we want these print statements here?
There was a problem hiding this comment.
Whoops... no, definitely not.
No description provided.