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-25208: Fix broken test in lsst/alert_packet master #20
Conversation
This helps avoid them getting listed in diffs, which is too messy to be useful.
dda4707
to
c3615d8
Compare
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.
I think that __eq__
is redundant, but assuming you agree (and remove it) I'm happy with this.
python/lsst/alert/packet/schema.py
Outdated
@@ -174,6 +174,10 @@ def __init__(self, schema_definition): | |||
self.definition = resolve_schema_definition(schema_definition) | |||
fastavro.schema._schema.SCHEMA_DEFS.clear() | |||
|
|||
|
|||
def __eq__(self, other): | |||
return self.definition == other.definition |
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.
This looks pretty similar to
def __eq__(self, other): |
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 sure does, doesn't it. I don't know why a particular test had been failing intermittently, then, but it doesn't seem to fail anymore.
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.
Incidentally, today I learned that you can declare a method twice inside one class. Why on earth is that allowed? Ah, Python.
c3615d8
to
e71ddd4
Compare
Ticket:
DM-25208
There were several issues:
Schema equality checks weren't quite right. I needed to add a(revealed in review to be redundant)__eq__
method to them.Along the way, I'm marking .avro files as "binary" so they don't show up in diffs as a bunch of garbage.