Skip to content

Updates for Windows & Ar 2.0: ResolvedPath, Win drive letters#29

Merged
mds-dwa merged 1 commit into
dreamworksanimation:python3from
garyo:python3
Jun 17, 2022
Merged

Updates for Windows & Ar 2.0: ResolvedPath, Win drive letters#29
mds-dwa merged 1 commit into
dreamworksanimation:python3from
garyo:python3

Conversation

@garyo
Copy link
Copy Markdown
Contributor

@garyo garyo commented Mar 21, 2022

  • Windows links to drive-based files need three leading slashes
  • Ar 2.0 no longer has AnchorRelativePath; replace w/ CreateIdentifier
  • expandPath returns a ResolvedPath that needs to be converted to
    string before being passed to python in a couple of places

Signed-off-by: Gary Oberbrunner garyo@darkstarsystems.com

- Windows links to drive-based files need three leading slashes
- Ar 2.0 no longer has `AnchorRelativePath`; replace w/ `CreateIdentifier`
- expandPath returns a `ResolvedPath` that needs to be converted to
  string before being passed to python in a couple of places

Signed-off-by: Gary Oberbrunner <garyo@darkstarsystems.com>
@mds-dwa
Copy link
Copy Markdown
Contributor

mds-dwa commented Mar 21, 2022

Hi @garyo, can you please submit a signed contributor license agreement per this page when you get a chance so that we can proceed? Overall, the changes look good, but we'll need to test on this end once we get the green light from legal. Thank you for your contribution!

@garyo
Copy link
Copy Markdown
Contributor Author

garyo commented Mar 23, 2022

CLA submitted just now.

@mds-dwa
Copy link
Copy Markdown
Contributor

mds-dwa commented Mar 23, 2022

Thank you! Once I get the OK from DWA, I’ll start testing internally.

@mds-dwa
Copy link
Copy Markdown
Contributor

mds-dwa commented May 23, 2022

Just so you don't think this got lost in the ether, I was out on leave for a good while and one of our people that needs to approve this internally is likewise out for a bit longer, but I'll try to get it pushed through in the next few weeks. Apologies for the lengthy delay!

Comment thread usdmanager/parsers/usd.py
def pathForLink(path):
"""Need three slashes before drive letter on Windows; this prepends one, so
with the usual two URL slashes we'll get the proper format."""
if os.name == 'nt' and re.match(r'^[a-zA-Z]:[/\\]', fullPath):
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

We should probably pre-compile this regex at the top of this module and re-use it, since pathForLink will get called a lot when large files are loaded.

Comment thread usdmanager/utils.py
context = resolver.CreateDefaultContextForAsset(path)
with Ar.ResolverContextBinder(context):
anchoredPath = path if parentPath is None else resolver.AnchorRelativePath(parentPath, path)
anchoredPath = path if parentPath is None else resolver.CreateIdentifier(path)
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I think we should do an "if" here similar to this to maintain compatibility with older asset resolvers. Thoughts?

@mds-dwa mds-dwa merged commit 6ca8f9f into dreamworksanimation:python3 Jun 17, 2022
@mds-dwa
Copy link
Copy Markdown
Contributor

mds-dwa commented Jun 17, 2022

I'm likely going to make some minor tweaks based on my above comments, but otherwise, this should be good to go. We'll be releasing a new 0.14 very soon.

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.

2 participants