Closed ctuguinay closed 6 days ago
All modified and coverable lines are covered by tests :white_check_mark:
Project coverage is 77.97%. Comparing base (
9f56124
) to head (41f987e
). Report is 55 commits behind head on main.
:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.
@leewujung This is ready for review. Small fix for a warning I kept encountering when trying to do the OOI notebook PR
@leewujung I made the change that you proposed in #1335. The failing tests are unrelated to this PR and are the processing level tests mentioned in #1342.
I am curious though, why in
EXPECTED_VAR_DTYPE = {
"channel": np.str_,
"cal_channel_id": np.str_,
"beam": np.str_,
"channel_mode": np.float16,
"beam_stabilisation": np.byte,
"non_quantitative_processing": np.int16,
} # channel name # beam name
are we only checking for these subset of variable types? Are they special in some way?
are we only checking for these subset of variable types? Are they special in some way?
In fact I don't remember exactly why. But I think it was due to similar warnings, since before these types were either not set or set to something that would cause a warning.
I think we should at some point implement a pydantic approach for all the types, to avoid the messiness and the arbitrary patching approach you pointed out here.
Also note that our NaN-padding approach probably made many of the resultant types different from those written in the convention. Something we should also discuss, probably with the user community and convention folks.
Addresses #1335. I have a step-by-step explanation of the problem and my proposed solution on the linked issue.