Fix metadata for islands queries #328
Conversation
| 'island_id': 'U', | ||
| 'island_name': 'U', | ||
| 'n_components': 'f', | ||
| 'ra_hms_cont': 'U', | ||
| 'dec_dms_cont': 'U', | ||
| 'ra_deg_cont': 'f', | ||
| 'dec_deg_cont': 'f', | ||
| 'freq': 'f', | ||
| 'maj_axis': 'f', | ||
| 'min_axis': 'f', | ||
| 'pos_ang': 'f', | ||
| 'flux_int': 'f', | ||
| 'flux_int_err': 'f', | ||
| 'flux_peak': 'f', | ||
| 'mean_background': 'f', | ||
| 'background_noise': 'f', | ||
| 'max_residual': 'f', | ||
| 'min_residual': 'f', | ||
| 'mean_residual': 'f', | ||
| 'rms_residual': 'f', | ||
| 'stdev_residual': 'f', | ||
| 'x_min': 'i', | ||
| 'x_max': 'i', | ||
| 'y_min': 'i', | ||
| 'y_max': 'i', | ||
| 'n_pix': 'i', | ||
| 'solid_angle': 'f', | ||
| 'beam_area': 'f', | ||
| 'x_ave': 'f', | ||
| 'y_ave': 'f', | ||
| 'x_cen': 'f', | ||
| 'y_cen': 'f', | ||
| 'x_peak': 'i', | ||
| 'y_peak': 'i', | ||
| 'flag_i1': 'i', | ||
| 'flag_i2': 'i', | ||
| 'flag_i3': 'i', | ||
| 'flag_i4': 'i', | ||
| 'comment': 'U', | ||
| 'detection': '?' |
There was a problem hiding this comment.
I'm wondering, do these different columns break anything further down the line? Is there a column missing that is explicitly referred to later on?
There was a problem hiding this comment.
You're right.
rms_image and flux_peak_err are missing which would break Source.write_measurements(simple=True). There is a background_noise which we could use to replace rms_image and we could probably also calculate flux_peak_error using the Condon et al. equations. On the other hand it might be better to just fill them with NaN values.
I've been thinking for some time that we really need some form of end-to-end testing that will pick up on this kind of thing, but I think it'd just be too hard to write.
There was a problem hiding this comment.
You're right.
rms_imageandflux_peak_errare missing which would breakSource.write_measurements(simple=True). There is abackground_noisewhich we could use to replacerms_imageand we could probably also calculateflux_peak_errorusing the Condon et al. equations. On the other hand it might be better to just fill them with NaN values.
If the end result is not too 'wrong', and for the sake of keeping it simple, I think yes, use what is already here to replace those columns. You might be able to get away with using background_noise for both? Just make sure it's documented in the docs or the code that this is happening.
I've been thinking for some time that we really need some form of end-to-end testing that will pick up on this kind of thing, but I think it'd just be too hard to write.
Yes it's very much only unit testing at the moment isn't it. It would probably snowball but it might be possible to get some sort of integration testing in there. You could move all the current tests under a unit directory i.e. tests/unit and then have an integration directory and test in there which does something end-to-end. The tricky bit is designing what that should be.
The notebooks always filled that void for me, but not ideal.
|
Added placeholders, warnings and docs. |
Fix #308.