Skip to content
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

Replace boost::totally_ordered with explicit operator overloads for SdfAssetPath #2293

Merged
merged 2 commits into from
Jul 3, 2023

Conversation

nvmkuruc
Copy link
Collaborator

@nvmkuruc nvmkuruc commented Feb 17, 2023

Description of Change(s)

  • Add explicit implementations of operator{<=,>,>=,!=} to SdfAssetPath
  • Expose copy constructor to python
  • Expose comparison operators to python
  • Add unit test coverage for Sdf.AssetPath operators

Fixes Issue(s)

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

@sunyab
Copy link
Contributor

sunyab commented Feb 17, 2023

Filed as internal issue #USD-8030

@@ -27,6 +27,84 @@
from pxr import Sdf, Tf
import unittest

Choose a reason for hiding this comment

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

I'm glad to see test cases added for SdfAssetPath, but I don't feel they belong in testSdfReference (it's not obvious to look for them here.) Can we instead add a new testSdfAssetPath to hold these tests? I'm a little surprised we don't have a testSdfAssetPath already.
Additionally, we typically make one unittest.TestCase class and have multiple test case methods for each case. That would make more sense here too for the trio of SdfAssetPath tests.

@pixar-oss pixar-oss merged commit 8883983 into PixarAnimationStudios:dev Jul 3, 2023
@nvmkuruc nvmkuruc deleted the sdfassetordered branch December 29, 2023 03:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants