Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Replace boost::equality_comparable with explicit implementation of operator!= in pxr/usd/sdf #2237

Merged
merged 2 commits into from
Jul 3, 2023

Conversation

nvmkuruc
Copy link
Collaborator

@nvmkuruc nvmkuruc commented Feb 2, 2023

Description of Change(s)

  • Adds explicit implementation of operator!= to SdfAllowed, SdfChildrenProxy, SdfNamespaceEdit, SdfNamespaceEditDetail, and SdfUnregisteredValue
  • Equality operator test coverage has been extended to Sdf.NamespaceEdit, Sdf.NamespaceEditDetail, and Sdf.UnregisteredValue.

Fixes Issue(s)

  • I have verified that all unit tests pass with the proposed changes
  • I have submitted a signed Contributor License Agreement

@tallytalwar
Copy link
Contributor

Filed as internal issue #USD-7965

@@ -146,6 +143,7 @@ struct SdfNamespaceEditDetail :
const std::string& reason);

SDF_API bool operator==(const SdfNamespaceEditDetail& rhs) const;

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just a nitpick. Since == uses "rhs", != should use "rhs" too

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done

Sdf.NamespaceEdit('/A', '/B'),
"reason"
)
self.assertTrue(prototype == prototype)

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The prototype == prototype test case would be more robust if it were
self.assertTrue(prototype == Sdf.NamespaceEditDetail(
 Sdf.NamespaceEditDetail.Okay,
 Sdf.NamespaceEdit('/A', '/B'),
 "reason"
 )
Same for !=

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done, I also switched the tests to use assertEqual and assertNotEqual which is preferred per https://docs.python.org/3/library/unittest.html#unittest.TestCase.assertFalse

@nvmkuruc nvmkuruc force-pushed the sdfequality branch 2 times, most recently from c4ef3f8 to 47e6cb3 Compare June 20, 2023 02:33
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants