-
Notifications
You must be signed in to change notification settings - Fork 202
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
Support Python3 for Pixar's UsdMaya and unit tests #555
Changes from 7 commits
3926f90
214e85d
ad8b01a
497d94c
66b2255
e594505
28f6222
389bc58
909c269
b7aad65
3de5818
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -47,6 +47,8 @@ def PrintError(error): | |
traceback.print_exc() | ||
print("ERROR:", error) | ||
|
||
def Python3(): | ||
return sys.version_info.major == 3 | ||
############################################################ | ||
def Windows(): | ||
return platform.system() == "Windows" | ||
|
@@ -558,6 +560,7 @@ def __init__(self, args): | |
Build directory {buildDir} | ||
Install directory {instDir} | ||
Variant {buildVariant} | ||
Python 3: {enablePython3} | ||
Python Debug {debugPython} | ||
CMake generator {cmakeGenerator}""" | ||
|
||
|
@@ -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"), | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Simple change to output Python3 state in the output console. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 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" There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @seando-adsk Correct, just a simple indication in the output console: There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think @seando-adsk's point was that that's indicating that There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 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. |
||
cmakeGenerator=("Default" if not context.cmakeGenerator | ||
else context.cmakeGenerator) | ||
) | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -239,21 +239,7 @@ find_program(MAYA_EXECUTABLE | |
"Maya's executable path" | ||
) | ||
|
||
find_program(MAYA_PY_EXECUTABLE | ||
mayapy | ||
HINTS | ||
"${MAYA_LOCATION}" | ||
"$ENV{MAYA_LOCATION}" | ||
"${MAYA_BASE_DIR}" | ||
PATH_SUFFIXES | ||
Maya.app/Contents/bin/ | ||
bin/ | ||
DOC | ||
"Maya's Python executable path" | ||
) | ||
|
||
if(MAYA_INCLUDE_DIRS AND EXISTS "${MAYA_INCLUDE_DIR}/maya/MTypes.h") | ||
|
||
# Tease the MAYA_API_VERSION numbers from the lib headers | ||
file(STRINGS ${MAYA_INCLUDE_DIR}/maya/MTypes.h TMP REGEX "#define MAYA_API_VERSION.*$") | ||
string(REGEX MATCHALL "[0-9]+" MAYA_API_VERSION ${TMP}) | ||
|
@@ -265,9 +251,29 @@ if(MAYA_INCLUDE_DIRS AND EXISTS "${MAYA_INCLUDE_DIR}/maya/MTypes.h") | |
else() | ||
string(SUBSTRING ${MAYA_API_VERSION} "0" "4" MAYA_APP_VERSION) | ||
endif() | ||
endif() | ||
|
||
# swtich between mayapy and mayapy2 | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Minor typo "switch" |
||
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" | ||
) | ||
|
||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Added a switch to handle mayapy executable name in dual Maya python. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Great. |
||
# handle the QUIETLY and REQUIRED arguments and set MAYA_FOUND to TRUE if | ||
# all listed variables are TRUE | ||
include(FindPackageHandleStandardArgs) | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -15,6 +15,8 @@ | |
# limitations under the License. | ||
# | ||
|
||
from future.utils import iteritems | ||
|
||
from pxr import UsdMaya | ||
|
||
from pxr import Tf | ||
|
@@ -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): | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. In python3, dict.iteritems is no longer available. Using python.future function iteritems() Note: mayapy shipped with Maya already comes with |
||
statsDict = { | ||
'profile': profileScopeName, | ||
'metric': 'time', | ||
|
@@ -131,7 +133,7 @@ def _RunPlaybackTest(self): | |
profileScopeName = '%s Proxy Orbit Playback Time' % self._testName | ||
|
||
with self._ProfileScope(profileScopeName): | ||
for frame in xrange(int(self.animStartTime), int(self.animEndTime + 1.0)): | ||
for frame in range(int(self.animStartTime), int(self.animEndTime + 1.0)): | ||
cmds.currentTime(frame, edit=True) | ||
|
||
playElapsedTime = self._profileScopeMetrics[profileScopeName] | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@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.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I had actually started looking into that once, but got side tracked. I think we should fix this schema stuff in a separate PR.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hey @seando-adsk see 389bc58