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

Setting VtArray elements via index in python does not work with [-1] but does work correctly with other negative indices #2327

Closed
nvmkuruc opened this issue Mar 10, 2023 · 4 comments

Comments

@nvmkuruc
Copy link
Collaborator

nvmkuruc commented Mar 10, 2023

Description of Issue

The python bindings for VtArray implements setting by index in terms of setting by slice, incrementing the index by 1 to create the slice. However, for -1, that creates an empty slice and the operation silently fails.

Steps to Reproduce

  1. Append the following test case to testVtArray.py
    def test_SetNegativeIndices(self):
        d = Vt.DoubleArray([1.0, 3.0, 5.0])
        d[-1] = 2.0
        d[-2] = 4.0
        self.assertEqual(d, [1.0, 4.0, 2.0])
  1. Build
  2. ctest -C Release -R testVtArray
    You should see the following error message.
    self.assertEqual(d, [1.0, 4.0, 2.0])
AssertionError: Vt.DoubleArray(3, (1, 4, 5)) != [1.0, 4.0, 2.0]

You can see that the second element from back was correctly set to 4.0 but the last element was not set to 2.0.

System Information (OS, Hardware)

Linux

Package Versions

Python 3.10
TBB 2020.3

Build Flags

--tests --no-imaging

@nvmkuruc nvmkuruc changed the title Setting values via index in VtArray in python does not work with [-1] but does work correctly with other negative indices Setting VtArray elements via index in python does not work with [-1] but does work correctly with other negative indices Mar 10, 2023
@sunyab
Copy link
Contributor

sunyab commented Mar 14, 2023

Filed as internal issue #USD-8088

@FlorianZ
Copy link
Contributor

@nvmkuruc good find! Are you interested in tackling this? We'd consider a contribution, for sure. Otherwise, we can pursue this internally, but it doesn't currently rise to the top of our internal priority list. Thank you!!

@nvmkuruc
Copy link
Collaborator Author

nvmkuruc commented Mar 14, 2023

Happy to submit one, but didn't want to start in case there was a strong opinion about the best fix. My instinct is to just special case -1 and convert it to [-1 + size(), size()).

@gitamohr
Copy link
Contributor

I have a fix for this, thanks for reporting, @nvmkuruc !

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

No branches or pull requests

4 participants