Support Python3 for Pixar's UsdMaya and unit tests #555

HamedSabri-adsk · 2020-06-02T20:33:40Z

This PR adds Python3 compatibility support for Pixar's MayaUSD and unit tests.

Note: Out of all tests, testUsdExportPackage is not passing right now in python 3.
See #555 (comment)

…a-usd in the clean build.

HamedSabri-adsk · 2020-06-02T20:34:52Z

build.py

@@ -588,6 +591,7 @@ def __init__(self, args):
        ctestArgs=context.ctestArgs,
        buildVariant=BuildVariant(context),
        debugPython=("On" if context.debugPython else "Off"),
+        enablePython3=("On" if Python3() else "Off"),


Simple change to output Python3 state in the output console.

This is only to indicate that the build.py was called with Python 3 - right? Not that we are actually building python 3 bindings. That is still controlled by the cmake option "BUILD_WITH_PYTHON_3"

@seando-adsk Correct, just a simple indication in the output console:

I think @seando-adsk's point was that that's indicating that build.py itself is running using Python 3, and not necessarily indicating that you're building for Python 3. You could run build.py using Python 3 but still be building maya-usd for Python 2, in which case that indicator might be misleading.

Yes I already realized that :) I agree with you that this could be misleading and in fact in my mind was thinking to change it. something for another day.

HamedSabri-adsk · 2020-06-02T20:38:00Z

CMakeLists.txt

-if (BUILD_USDMAYA_SCHEMAS)
-    include(cmake/jinja.cmake)
-endif()
-


@seando-adsk This was a bit of a surprise since this code never is called when the clean build happens.

BUILD_USDMAYA_SCHEMAS is an option that is set in AL. This value is not cached until AL's CmakeLists is called which happens later.

I think what happened is when we moved some schema stuff from AL into core we moved this. But we forgot to move the option. Shouldn't we leave this here and move the option under line 35?

I had actually started looking into that once, but got side tracked. I think we should fix this schema stuff in a separate PR.

Agree, I had to take it out eventually since I was getting bunch of weird issues with Python 3 when doing incremental builds. Let's revisit this together.

I realized that this needs to remain here. From the Maya build and our internal ecg-maya-usd repo we are setting this flag "BUILD_USDMAYA_SCHEMAS=ON" and providing the jinja/markupsafe locations. The jinja.cmake will check to see if jinja2 and markupsafe are part of the python executable. If not, then it will add them to the python path from the locations provided.

Hey @seando-adsk see 389bc58

HamedSabri-adsk · 2020-06-02T20:39:41Z

cmake/modules/FindMaya.cmake

+# swtich between mayapy and mayapy2
+set(mayapy_exe mayapy)
+if(${MAYA_APP_VERSION} STRGREATER_EQUAL "2021")
+    if(NOT BUILD_WITH_PYTHON_3)
+        set(mayapy_exe mayapy2)
+    endif()
 endif()

+find_program(MAYA_PY_EXECUTABLE
+        ${mayapy_exe}
+    HINTS
+        "${MAYA_LOCATION}"
+        "$ENV{MAYA_LOCATION}"
+        "${MAYA_BASE_DIR}"
+    PATH_SUFFIXES
+        Maya.app/Contents/bin/
+        bin/
+    DOC
+        "Maya's Python executable path"
+)
+


Added a switch to handle mayapy executable name in dual Maya python.

plugin/pxr/cmake/macros/Public.cmake

HamedSabri-adsk · 2020-06-02T20:50:44Z

plugin/pxr/maya/lib/pxrUsdMayaGL/testenv/testProxyShapeDrawPerformance.py

@@ -47,7 +49,7 @@ def setUpClass(cls):
    @classmethod
    def tearDownClass(cls):
        statsOutputLines = []
-        for profileScopeName, elapsedTime in cls._profileScopeMetrics.iteritems():
+        for profileScopeName, elapsedTime in iteritems(cls._profileScopeMetrics):


In python3, dict.iteritems is no longer available. Using python.future function iteritems()

Note: mayapy shipped with Maya already comes with future package installed

HamedSabri-adsk · 2020-06-02T20:55:50Z

plugin/pxr/maya/lib/usdMaya/testenv/testUsdExportPackage.py

@@ -24,7 +23,6 @@

 from pxr import Usd, UsdGeom

-


@mattyjams This test is still failing in Python3 on Windows:

The reason it fails is because GetFileNames which is simply a list of strings is different from the expected list.

zipFile = Usd.ZipFile.Open(usdFile) fileNames = zipFile.GetFileNames() self.assertEqual(fileNames, [ "MyAwesomePackage.usdc", "ReferenceModel.usda", "BaseModel.usda", "card.png" ])

After some debugging , I noticed the list entries in the python 3 version have the path to temp folder which is really bizarre:

['Users/sabrih/AppData/Local/Temp/tmpp7c6l4yj/MyAwesomePackage.usdc', 'Users/sabrih/AppData/Local/Temp/tmpp7c6l4yj/ReferenceModel.usda', 'Users/sabrih/AppData/Local/Temp/tmpp7c6l4yj/BaseModel.usda', 'Users/sabrih/AppData/Local/Temp/tmpp7c6l4yj/card.png']

VS the python 2 version doesn't have this temporary paths:

['MyAwesomePackage.usdc', 'ReferenceModel.usda', 'BaseModel.usda', 'card.png']

To me it looks like something to do with UsdZipFile.

Hmm, if that's the case, then it seems like that might be an issue with core USD?

Can you reproduce this in a non-Maya context, and if so, could you open an issue against the core USD GitHub project, if so?

@mattyjams see 28f6222

I ran part of the test in question in a non-Maya context both in Python 2 and 3. In my test I am not calling cmds.usdExport and as it shown below in both cases fileNames do contain temporary paths in the names. So I don't think this has anything to do with UsdZipFile.

I suspect it either has something to do with os.path.abspath or the call to usdExport. For now I have disabled this assert for Python 3 until further investigation.

Python 3

Python 3.7.7 (tags/v3.7.7:d7c567b08f, Mar 10 2020, 10:41:24) [MSC v.1900 64 bit (AMD64)] on win32 Type "help", "copyright", "credits" or "license" for more information. >>> import os >>> from pxr import Usd, UsdGeom >>> usdFile = os.path.abspath('C:\\Users\\sabrih\\AppData\\Local\\Temp\\tmpjwhzno7b\\MyAwesomePackage.usdz') >>> zipFile = Usd.ZipFile.Open(usdFile) >>> fileNames = zipFile.GetFileNames() >>> print(fileNames) ['Users/sabrih/AppData/Local/Temp/tmpjwhzno7b/MyAwesomePackage.usdc', 'Users/sabrih/AppData/Local/Temp/tmpjwhzno7b/ReferenceModel.usda', 'Users/sabrih/AppData/Local/Temp/tmpjwhzno7b/BaseModel.usda', 'Users/sabrih/AppData/Local/Temp/tmpjwhzno7b/card.png']

Python 2

Python 2.7.15 (v2.7.15:ca079a3ea3, Apr 30 2018, 16:30:26) [MSC v.1500 64 bit (AMD64)] on win32 Type "help", "copyright", "credits" or "license" for more information. >>> import os >>> from pxr import Usd, UsdGeom >>> usdFile = os.path.abspath('C:\\Users\\sabrih\\AppData\\Local\\Temp\\tmpjwhzno7b\\MyAwesomePackage.usdz') >>> zipFile = Usd.ZipFile.Open(usdFile) >>> fileNames = zipFile.GetFileNames() >>> print(fileNames) ['Users/sabrih/AppData/Local/Temp/tmpjwhzno7b/MyAwesomePackage.usdc', 'Users/sabrih/AppData/Local/Temp/tmpjwhzno7b/ReferenceModel.usda', 'Users/sabrih/AppData/Local/Temp/tmpjwhzno7b/BaseModel.usda', 'Users/sabrih/AppData/Local/Temp/tmpjwhzno7b/card.png']

HamedSabri-adsk · 2020-06-02T20:58:04Z

plugin/pxr/maya/lib/usdMaya/testenv/testUsdExportPointInstancer.py

+
+        # assertCountEqual in python 3 is equivalent to assertItemsEqual
+        if sys.version_info[0] >= 3:
+            self.assertItemsEqual = self.assertCountEqual


Python 3 no longer has assertItemsEqual and instead this function has been renamed to assertCountEqual.

added self.assertItemsEqual = self.assertCountEqual for compatibility between Python 2 and Python 3

mattyjams

Just one specific change that I think we also need in the testProxyShapeDuplicatePerformance test, and a few other minor little notes, but otherwise looks good to me!

plugin/pxr/maya/lib/pxrUsdMayaGL/testenv/testProxyShapeDuplicatePerformance.py

mattyjams · 2020-06-02T23:44:42Z

plugin/pxr/maya/lib/usdMaya/testenv/testUsdExportPackage.py

@@ -24,7 +23,6 @@

 from pxr import Usd, UsdGeom

-


Hmm, if that's the case, then it seems like that might be an issue with core USD?

Can you reproduce this in a non-Maya context, and if so, could you open an issue against the core USD GitHub project, if so?

plugin/pxr/maya/lib/usdMaya/testenv/testUsdImportXforms.py

plugin/pxr/maya/lib/usdMaya/testenv/testUsdMayaDiagnosticDelegate.py

mattyjams

Looks good!

I'm not sure how you want to handle the testUsdExportPackage failure. Maybe for now, just detect when it's running in Python 3 and bypass the problematic assert? If it does look like an issue in core USD, we can put a XXX or TODO or whatever comment there tying it to the GitHub issue.

seando-adsk · 2020-06-03T16:55:05Z

CMakeLists.txt

-if (BUILD_USDMAYA_SCHEMAS)
-    include(cmake/jinja.cmake)
-endif()
-


I think what happened is when we moved some schema stuff from AL into core we moved this. But we forgot to move the option. Shouldn't we leave this here and move the option under line 35?

seando-adsk · 2020-06-03T16:57:02Z

build.py

@@ -588,6 +591,7 @@ def __init__(self, args):
        ctestArgs=context.ctestArgs,
        buildVariant=BuildVariant(context),
        debugPython=("On" if context.debugPython else "Off"),
+        enablePython3=("On" if Python3() else "Off"),


This is only to indicate that the build.py was called with Python 3 - right? Not that we are actually building python 3 bindings. That is still controlled by the cmake option "BUILD_WITH_PYTHON_3"

seando-adsk · 2020-06-03T16:57:37Z

cmake/modules/FindMaya.cmake

+# swtich between mayapy and mayapy2
+set(mayapy_exe mayapy)
+if(${MAYA_APP_VERSION} STRGREATER_EQUAL "2021")
+    if(NOT BUILD_WITH_PYTHON_3)
+        set(mayapy_exe mayapy2)
+    endif()
 endif()

+find_program(MAYA_PY_EXECUTABLE
+        ${mayapy_exe}
+    HINTS
+        "${MAYA_LOCATION}"
+        "$ENV{MAYA_LOCATION}"
+        "${MAYA_BASE_DIR}"
+    PATH_SUFFIXES
+        Maya.app/Contents/bin/
+        bin/
+    DOC
+        "Maya's Python executable path"
+)
+


seando-adsk · 2020-06-03T16:59:46Z

CMakeLists.txt

-if (BUILD_USDMAYA_SCHEMAS)
-    include(cmake/jinja.cmake)
-endif()
-


I had actually started looking into that once, but got side tracked. I think we should fix this schema stuff in a separate PR.

seando-adsk · 2020-06-03T20:29:09Z

CMakeLists.txt

-if (BUILD_USDMAYA_SCHEMAS)
-    include(cmake/jinja.cmake)
-endif()
-


I realized that this needs to remain here. From the Maya build and our internal ecg-maya-usd repo we are setting this flag "BUILD_USDMAYA_SCHEMAS=ON" and providing the jinja/markupsafe locations. The jinja.cmake will check to see if jinja2 and markupsafe are part of the python executable. If not, then it will add them to the python path from the locations provided.

…when maya-usd in the clean build." This reverts commit 214e85d.

…py_exe

seando-adsk · 2020-06-05T18:12:45Z

cmake/modules/FindMaya.cmake


+# swtich between mayapy and mayapy2


Minor typo "switch"

kxl-adsk

Leaving a comment that we have to address Maya 2019 test failures.

The following tests FAILED:
         20 - testUsdExportEulerFilter (Failed)
         21 - testUsdExportFilterTypes (Failed)
         45 - testUsdImportCamera (Failed)
         58 - testUsdImportXforms (Failed)
         73 - testUsdMayaXformStack (Failed)
Errors while running CTest

kxl-adsk

I was able to validate that failing tests are passing once Python's future package is installed. What's left to do before we can merge this PR is to document this new requirement for running tests in https://github.com/Autodesk/maya-usd/blob/dev/doc/build.md

FWIW. this is the process I used to get tests running for both Maya 2019 and 2020:
1 - Download get-py.py from https://bootstrap.pypa.io/get-pip.py
2 - Open a command-line and navigate to mayapy.exe for Maya version which needs future package. Run:
mayapy.exe <path_to_downloaded_file>/get-pip.py
3 - Pip will be added to <path_to_maya>Python/Scripts
4 - Install the future package
<path_to_maya>Python/Scripts/pip install future

HamedSabri-adsk · 2020-06-15T12:43:30Z

I was able to validate that failing tests are passing once Python's future package is installed. What's left to do before we can merge this PR is to document this new requirement for running tests in https://github.com/Autodesk/maya-usd/blob/dev/doc/build.md

Thank you @kxl-adsk for the steps. Please see b7aad65

…nstalled manually.

kxl-adsk

Thank you @HamedSabri-adsk - LGTM!

Hamed Sabri added 5 commits June 2, 2020 16:21

Output Python3 ON/OFF in build script.

3926f90

Remove include(cmake/jinja.cmake). This code never is called when may…

214e85d

…a-usd in the clean build.

Handle switch between mayapy and mayapy2

ad8b01a

There is no need to install __init__.pyc

497d94c

make python tests compatible with python 3.

66b2255

HamedSabri-adsk added pxr Related to Pixar plugin unit test Related to unit tests (both python or c++) labels Jun 2, 2020

HamedSabri-adsk requested review from mattyjams and seando-adsk June 2, 2020 20:33

HamedSabri-adsk commented Jun 2, 2020

View reviewed changes

plugin/pxr/cmake/macros/Public.cmake Show resolved Hide resolved

HamedSabri-adsk commented Jun 2, 2020

View reviewed changes

kxl-adsk assigned ecp-maya-devops-adsk Jun 2, 2020

mattyjams suggested changes Jun 2, 2020

View reviewed changes

Address feedbacks.

e594505

HamedSabri-adsk requested a review from mattyjams June 3, 2020 00:33

mattyjams approved these changes Jun 3, 2020

View reviewed changes

kxl-adsk unassigned ecp-maya-devops-adsk Jun 3, 2020

For now, disable assertEqual that causes the test fail in python 3.

28f6222

seando-adsk approved these changes Jun 3, 2020

View reviewed changes

seando-adsk requested changes Jun 3, 2020

View reviewed changes

Revert "Remove include(cmake/jinja.cmake). This code never is called …

389bc58

…when maya-usd in the clean build." This reverts commit 214e85d.

HamedSabri-adsk requested a review from seando-adsk June 4, 2020 20:43

Add a check to make sure mayapy2 executable exist before setting maya…

909c269

…py_exe

HamedSabri-adsk assigned ecp-maya-devops-adsk Jun 4, 2020

kxl-adsk unassigned ecp-maya-devops-adsk Jun 4, 2020

seando-adsk reviewed Jun 5, 2020

View reviewed changes

cmake/modules/FindMaya.cmake

# swtich between mayapy and mayapy2

Copy link

Collaborator

seando-adsk Jun 5, 2020

Choose a reason for hiding this comment

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

Minor typo "switch"

seando-adsk approved these changes Jun 5, 2020

View reviewed changes

kxl-adsk suggested changes Jun 5, 2020

View reviewed changes

kxl-adsk suggested changes Jun 13, 2020

View reviewed changes

update build doc with instruction for installing future package.

b7aad65

Add 2018 to the list of Maya versions that needs the future package i…

3de5818

…nstalled manually.

HamedSabri-adsk requested a review from kxl-adsk June 15, 2020 13:19

kxl-adsk approved these changes Jun 15, 2020

View reviewed changes

kxl-adsk merged commit 1a5d926 into dev Jun 15, 2020

kxl-adsk deleted the sabrih/MAYA-104922/support_python3_pxr branch June 15, 2020 13:30

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Support Python3 for Pixar's UsdMaya and unit tests #555

Support Python3 for Pixar's UsdMaya and unit tests #555

HamedSabri-adsk commented Jun 2, 2020 •

edited

Loading

HamedSabri-adsk Jun 2, 2020

seando-adsk Jun 3, 2020

HamedSabri-adsk Jun 3, 2020

mattyjams Jun 3, 2020

HamedSabri-adsk Jun 3, 2020

HamedSabri-adsk Jun 2, 2020

seando-adsk Jun 3, 2020

seando-adsk Jun 3, 2020

HamedSabri-adsk Jun 3, 2020

seando-adsk Jun 3, 2020

HamedSabri-adsk Jun 4, 2020

HamedSabri-adsk Jun 2, 2020

seando-adsk Jun 3, 2020

HamedSabri-adsk Jun 2, 2020

HamedSabri-adsk Jun 2, 2020

mattyjams Jun 2, 2020

HamedSabri-adsk Jun 3, 2020

HamedSabri-adsk Jun 2, 2020

mattyjams left a comment

mattyjams Jun 2, 2020

mattyjams left a comment

seando-adsk Jun 3, 2020

seando-adsk Jun 3, 2020

seando-adsk Jun 3, 2020

seando-adsk Jun 3, 2020

seando-adsk Jun 3, 2020

seando-adsk Jun 5, 2020

kxl-adsk left a comment

kxl-adsk left a comment •

edited

Loading

HamedSabri-adsk commented Jun 15, 2020

kxl-adsk left a comment

Support Python3 for Pixar's UsdMaya and unit tests #555

Support Python3 for Pixar's UsdMaya and unit tests #555

Conversation

HamedSabri-adsk commented Jun 2, 2020 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mattyjams left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mattyjams left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

kxl-adsk left a comment

Choose a reason for hiding this comment

kxl-adsk left a comment • edited Loading

Choose a reason for hiding this comment

HamedSabri-adsk commented Jun 15, 2020

kxl-adsk left a comment

Choose a reason for hiding this comment

HamedSabri-adsk commented Jun 2, 2020 •

edited

Loading

kxl-adsk left a comment •

edited

Loading