New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
DM-33221: Add DiffMatchedTractCatalogTask #634
Conversation
0a315a1
to
a31bc6b
Compare
---------- | ||
values : `Collection` [`float`] | ||
A set of values to compute the statistic for. | ||
Returns |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you put a line break above returns to make it more obvious that it isn't a parameter?
|
||
Returns | ||
------- | ||
row_stats : `numpy.ndarray`, (1, C) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
row_with_stats? Make it more obvious that this is the original row with the stats and not just the stats for the row.
retStruct = pipeBase.Struct(cat_matched=cat_matched) | ||
# Add/compute distance columns | ||
coord1_target_err, coord2_target_err = config.columns_target_coord_err | ||
column_dist, column_dist_err = 'dr', 'drErr' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is this the name that the columns end up with in the table? If so could it be something more descriptive?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It was meant to be consistent with the default (pixel) coordinate naming of x and y, but I suppose coord_x
,coord_y
and distance
would be clearer.
|
||
band_prev = None | ||
for idx_band, (band, config_flux) in enumerate(config.columns_flux.items()): | ||
mag_ref = -2.5 * np.log10(cat_ref[config_flux.column_ref_flux]) + config.mag_zeropoint_ref |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Spaces around *
column_color_err_temp_model = f'{column_color_err_temp}{idx_model}' | ||
|
||
# e.g. if order is ugrizy, first color will be u - g | ||
cat_target[column_color_temp_model] = mag_prev[idx_model] - mag_model[idx_model] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Are they guaranteed to be in order?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
They're in config.columns_flux order, so the user can specify the N-1 colours they want. I suppose I could output all N*(N-1)/2 unique colours instead, since that's "only" 15 instead of 5. I'll leave that for a future hypothetical feature request.
a31bc6b
to
def94d6
Compare
c767e8d
to
a987543
Compare
a987543
to
568ee10
Compare
No description provided.