-
Notifications
You must be signed in to change notification settings - Fork 50
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
Allowing to pass multiple unique attributes for tags in a dictionary #61
Conversation
e8b6b00
to
11ca642
Compare
Pull Request Test Coverage Report for Build 233
💛 - Coveralls |
11ca642
to
18460ae
Compare
18460ae
to
045796b
Compare
OK, that sounds reasonable, I won't get time to look at in detail until a few weeks though. |
No problem, thank you very much for your consideration and taking the time to review! |
if attr in left.attrib or attr in right.attrib: | ||
# One of the nodes have a unique attribute, we check only that. | ||
if isinstance(attr, dict): | ||
for tag, attr_list in attr.items(): |
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.
We could shortcut the whole test here, if we check that the tags are the same before the loop.
Then you also need only if tag != left.tag
below, as the tags now are known to be the same.
if attr in left.attrib or attr in right.attrib] | ||
for inner_attr in attr_list_consolidated: | ||
matches += int(left.attrib.get(inner_attr) | ||
== right.attrib.get(inner_attr)) |
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.
You could also shortcut this here. You don't need to check all attributes unless the match. Find one that doesn't match, and it's not a match. So no need to count matches.
# If only one node has it, it means they are not the same. | ||
return int(left.attrib.get(attr) == right.attrib.get(attr)) | ||
match = int(left.attrib.get(attr) == right.attrib.get(attr)) | ||
if match is None: |
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.
No, this is wrong. If one side has a unique attribute, it's either a full match or not a match. Nothing else needs to be checked. You can loop through the unique attributes, and if one side has one, and the other don't, then you can exit the whole function with a 0.
Extending the option 'uniqueattrs' by allowing to pass a dictionary where key denotes the tagname and value denotes a list of attributes which must be equal for nodes to match. This extends the current (tag, attribute)-tuple by providing the possibility to have multiple attributes which must be equal for a match. Helpful for XML represented databases where primary keys consist of multilple fields