Improving the compare_entities functions

Summary

I reviewed the compare_entities function from apiutils and identified some problematic cases (e.g. multi-properties). Corresponding unit tests have been created and the functionality repaired.

This is a preparation for a comprehensive review of the merge_entities function. I think it's a good idea to merge this in a first step as it's probably independent from the review of the merge function.

Focus

This one can probably be reviewed line-by-line. Maybe look for missing test-corner-cases that might be needed. The code for checking the value for equality is still not ideal, but I think a more comprehensive review (also adding the recursive reference checker) should be postponed.

Test Environment

There are unit tests.

Check List for the Author

Please, prepare your MR for a review. Be sure to write a summary and a focus and create gitlab comments for the reviewer. They should guide the reviewer through the changes, explain your changes and also point out open questions. For further good practices have a look at our review guidelines

• All automated tests pass
• Reference related issues
• Up-to-date CHANGELOG.md (or not necessary)
• Up-to-date JSON schema (or not necessary)
• Appropriate user and developer documentation (or not necessary)
  • How do I use the software? Assume "stupid" users.
  • How do I develop or debug the software? Assume novice developers.
• Annotations in code (Gitlab comments)
  • Intent of new code
  • Problems with old code
  • Why this implementation?

Check List for the Reviewer

• I understand the intent of this MR
• All automated tests pass
• Up-to-date CHANGELOG.md (or not necessary)
• Appropriate user and developer documentation (or not necessary)
• The test environment setup works and the intended behavior is reproducible in the test environment
• In-code documentation and comments are up-to-date.
• Check: Are there specifications? Are they satisfied?

For further good practices have a look at our review guidelines.

requested review from @henrik

assigned to @salexan

added 2 commits

• f70a3716 - MAINT: replaced xor operator with != for readability
• bceb49a9 - ENH: added missing check for multi-properties and added corresponding test cases

Compare with previous version

added 2 commits

• 15393549 - MAINT: moved check for multi-properties for clarity
• 63658ad0 - FIX: additional checks for unsupported multi-properties were needed

Compare with previous version

marked this merge request as ready

changed title from Draft: Improving the compare_entities and merge_entities functions to Improving the compare_entities functions

changed the description

requested review from @florian and removed review request for @henrik

marked the checklist item All automated tests pass as completed

added 1 commit

• 37bb9562 - DOC: updated changelog

Compare with previous version

marked the checklist item Up-to-date CHANGELOG.md (or not necessary) as completed

marked the checklist item Appropriate user and developer documentation (or not necessary) as completed

marked the checklist item Up-to-date JSON schema (or not necessary) as completed

mentioned in issue #164 (closed)

added 1 commit

• 4ebec82b - STY: autopep8'd

Compare with previous version

marked the checklist item I understand the intent of this MR as completed

added 6 commits

• 4ebec82b...08a4e13f - 5 commits from branch dev
• a9c09d73 - Merge branch 'dev' into f-check-merge-entities

Compare with previous version

added 1 commit

• 46709d59 - DOC: Update changelog

Compare with previous version

marked the checklist item Up-to-date CHANGELOG.md (or not necessary) as completed

marked the checklist item Appropriate user and developer documentation (or not necessary) as completed

marked the checklist item The test environment setup works and the intended behavior is reproducible in the test as completed

marked the checklist item In-code documentation and comments are up-to-date. as completed

approved this merge request

enabled an automatic merge when the pipeline for 46709d59 succeeds

aborted the automatic merge because source branch was updated

added 1 commit

• 3515d1da - STY: autopep8'd

Compare with previous version

enabled an automatic merge when the pipeline for 3515d1da succeeds

mentioned in commit 5ce8e910

merged

mentioned in commit f4e2aff0

mentioned in merge request !106