DM-47120: Add more columns to metric viewer #10
Conversation
| return (patch_str,) | ||
|
|
||
|
|
||
| def mk_shape_cols(t, metric_defs, n, bands, col_dict): |
There was a problem hiding this comment.
I would replace col_dict with a list of columns, and have the caller be responsible extracting the shape_cols entry from the dictionary.
(Same thing applies to the other functions that use col_dict, but I'm not going to repeat the comment each time)
| sig_str = f"<FONT CLASS=badValue>{sig:.3g}</FONT>\n" | ||
| bad_val += 1 | ||
|
|
||
| return val_str, sig_str, bad_val, link, debug_group |
There was a problem hiding this comment.
I would replace this tuple with a named dataclass.
| shape_str += sig_str + "<BR>\n" | ||
| if debug_group is not None: | ||
| debug_group_all = debug_group | ||
| shape_strs.append((shape_str, num_bad, link, debug_group_all)) |
There was a problem hiding this comment.
This tuple too should be a named class (same applies to other uses of this set of return values).
| import numpy as np | ||
|
|
||
|
|
||
| def mk_table_value(t, metric_defs, val_col_name, sig_col_name, n): |
There was a problem hiding this comment.
I'm not keen on passing a table and a row number, can this take a row instead? The other option in my mind is to take the values and the column name; I think that might be even cleaner but I'm not positive (i.e. take val and sig instead of extracting them in lines 64, 66 etc).
| The formatted number of patches in each band | ||
| """ | ||
| patch_str = "" | ||
| for band in ["g", "r", "i", "z", "y"]: |
There was a problem hiding this comment.
ok with this being hard coded but a variable at the top of the file is probably better. Should add u. All the instances in other functions should reference the variable too.
| patch_col = "coaddPatchCount_" + band + "_patchCount" | ||
| patch_str += "<B>" + band + "</B>: " + str(int(t[patch_col][n])) + "<BR>\n" | ||
|
|
||
| return (patch_str,) |
| __name__, | ||
| url_prefix="/plot-navigator/metrics", | ||
| static_folder="../../../../static", | ||
| ) | ||
|
|
||
| NO_BUTLER = True |
|
|
||
| # Add a nan/bad summary cell next but need to calculate these numbers first | ||
| row_list.append((num_bad,)) |
| {% else %} | ||
| {% if 'badValue' in cell[0] %} | ||
| <td> | ||
| <a href={{url_for(cell[1], collection_name=collection_urlencoded, metric=cell[2])}} |
There was a problem hiding this comment.
My hope is that using a dataclass for cell will make it so you can use named fields instead of [1], [2], etc? I assume Jinja will be ok with that, but I'm not sure.
| ) | ||
| if val_str is None: | ||
| continue | ||
| row_str += ( |
There was a problem hiding this comment.
Would this be simpler as an f-string?
sr525
force-pushed
the
tickets/DM-47120
branch
from
024482d to
e2e8712
Compare
No description provided.