Enable equality operator on Visibilities and VisMeta classes #64
Conversation
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #64 +/- ##
=======================================
Coverage ? 88.57%
=======================================
Files ? 7
Lines ? 875
Branches ? 0
=======================================
Hits ? 775
Misses ? 100
Partials ? 0 ☔ View full report in Codecov by Sentry. |
|
This should make Paolo's testing work easier. |
xrayvision/visibility.py
Outdated
| and np.allclose(self.time_range.mjd == other.time_range.mjd) | ||
| ): | ||
| return False | ||
| return True |
There was a problem hiding this comment.
What about the other keys which could set since the object inherits from dict?
There was a problem hiding this comment.
There are a couple justifications for not checking those:
- Two metas whose
VisMetaABCattributes are equal, are equalVisMetaABCobjects. Values outside the definition of theABCare out of scope. - Since these values aren't defined, and given the flexibility of types that
dictcan take, there isn't an obvious generalised way to check that the values are equal. Some may requireval1 == val2, others may requireall(val1 == val2)orassert_quantity_allclose(val1, val2)orassert np.allclose(val1, val2), or perhaps something more custom for custom objects. Given this, it's well-defined to stick to just theVisMetaABC-defined attributes.
There was a problem hiding this comment.
As long as we clearly document sounds good.
| """ | ||
| Checks whether two VisMeta objects are equal. | ||
|
|
||
| Does not check whether their metas are equal. |
There was a problem hiding this comment.
So this is really checking avis.data == bvis.data but we don't have a .data? What is the rational for not checking the meta.
There was a problem hiding this comment.
The thought is that even if the metadata is different, the visibilities themselves could be the same. For example, if the source is invariant over time or energy, then the visibilities should be the same, even it the metadata is different.
This decision though is open to discussion.
There was a problem hiding this comment.
I'm just trying to think of other python, sunpy or astropy etc object that work this can see arguments both ways - I guess it not blocker at this point though.
There was a problem hiding this comment.
Ok so the only two place I can see something similar do opposite things sunpy.timeseries (https://github.com/sunpy/sunpy/blob/50f4588b53b1d575623dfc8a2536595e76261746/sunpy/timeseries/timeseriesbase.py#L870) compares the meta where as astropy.table (https://github.com/astropy/astropy/blob/8c21018c0f0d9c7de38ed4f30845f9ae0f825b94/astropy/table/table.py#L3818) only compares the rows. So again once we document think all good.
No description provided.