Skip to content

Add support for removing version AND (not or) framenumber. #68

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.

Sign up for GitHub

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

Open
reformstudios wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Add support for removing version AND (not or) framenumber. #68

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
26 changes: 24 additions & 2 deletions hooks/path_info.py
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,3 +1,7 @@
'''
Copy link
Contributor

@josh-t josh-t Apr 11, 2018

Choose a reason for hiding this comment

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

guessing this was left by accident?

Copy link
Author

@reformstudios reformstudios Apr 11, 2018

Choose a reason for hiding this comment

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

indeed.

(.*)([._-])v(.*)([._-])(\d+)\.([^.]+)$
'''

# Copyright (c) 2017 Shotgun Software Inc.
#
# CONFIDENTIAL AND PROPRIETARY
Expand All @@ -24,18 +28,24 @@
VERSION_REGEX = re.compile("(.*)([._-])v(\d+)\.?([^.]+)?$", re.IGNORECASE)

# a regular expression used to extract the frame number from the file.
# this implementation assumes the version number is of the form '.####'
# this implementation assumes the frame number is of the form '.####'
Copy link
Contributor

@josh-t josh-t Apr 11, 2018

Choose a reason for hiding this comment

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

nice catch! my cut and paste error.

# coming just before the extension in the filename and just after a '.', '_',
# or '-'.
FRAME_REGEX = re.compile("(.*)([._-])(\d+)\.([^.]+)$", re.IGNORECASE)

# a regular expression used to extrand the frame number AND version from the file.
Copy link
Contributor

@josh-t josh-t Apr 11, 2018

Choose a reason for hiding this comment

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

typo: extrand => extract

Copy link
Author

@reformstudios reformstudios Apr 11, 2018

Choose a reason for hiding this comment

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

Oops!

# this implementation assumes the frame number is of the form '.####'
# coming just before the extension in the filename and just after a '.', '_',
# or '-'.
# also assuming the version number is of the form 'v###' coming before the frame number and frame seperator.
VERSION_FRAME_REGEX = re.compile("(.*)([._-])v(.*)([._-])(\d+)\.([^.]+)$", re.IGNORECASE)

class BasicPathInfo(HookBaseClass):
"""
Methods for basic file path parsing.
"""

def get_publish_name(self, path, sequence=False):
def get_publish_name(self, path, sequence=False, version=False):
Copy link
Contributor

@josh-t josh-t Apr 11, 2018

Choose a reason for hiding this comment

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

suggest making the new arg strip_frame=False or something like that.

Copy link
Author

@reformstudios reformstudios Apr 11, 2018

Choose a reason for hiding this comment

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

True, my suggestion is ambiguous... but do you mean 'strip_version' in this case?... because currently, with sequence = False, the version is removed, but if sequence=True, then the version remains. So we need both strip_version and strip_frame I think... and to cover all bases you might as well add a strip_ext option too!

Copy link
Contributor

@josh-t josh-t Apr 11, 2018

Choose a reason for hiding this comment

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

My preference here would be to include the most common behaviors, keeping the interface fairly clean. Now that I'm looking at this again, maybe we should have just done something like this from the start:

  • if this is a sequence, remove the frame number
  • try to remove any version number (acting on the result of ^ if applicable)

Maybe that would be sufficient and keep things simple (for most cases) and consistent. Anything beyond that would imply taking over the hook. Curious what you think.

"""
Given a file path, return the display name to use for publishing.

Expand Down Expand Up @@ -70,6 +80,7 @@ def get_publish_name(self, path, sequence=False):

version_pattern_match = re.search(VERSION_REGEX, filename)
frame_pattern_match = re.search(FRAME_REGEX, filename)
version_frame_pattern_match = re.search(VERSION_FRAME_REGEX, filename)
Copy link
Contributor

@josh-t josh-t Apr 11, 2018

Choose a reason for hiding this comment

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

Maybe we don't need the additional regex? Can we make this a 2-pass process if the additional arg is supplied?

if the assumption is that the frame number is always after the version number (when both are included) maybe the logic should be:

if strip_frame and frame_pattern_match:
    # remove the frame number with the existing frame regex

# continue with the existing logic...

Copy link
Author

@reformstudios reformstudios Apr 11, 2018

Choose a reason for hiding this comment

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

I think this is the best approach. I did consider that this might be possible, but in the end went with the simple option of formulating a specific regex.


if version_pattern_match:
# found a version number, use the other groups to remove it
Expand All @@ -88,6 +99,17 @@ def get_publish_name(self, path, sequence=False):
extension = frame_pattern_match.group(4) or ""
publish_name = "%s%s%s.%s" % (
prefix, frame_sep, display_str, extension)
elif version_frame_pattern_match and sequence and version:
Copy link
Contributor

@josh-t josh-t Apr 11, 2018

Choose a reason for hiding this comment

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

See comments above.

# found a frame number, meplace it with #s
prefix = version_frame_pattern_match.group(1)
version_sep = version_frame_pattern_match.group(2)
version = version_frame_pattern_match.group(3)
frame_sep = version_frame_pattern_match.group(4)
frame = version_frame_pattern_match.group(5)
display_str = "#" * len(frame)
extension = version_frame_pattern_match.group(6) or ""
publish_name = "%s%s%s.%s" % (
prefix, frame_sep, display_str, extension)
else:
publish_name = filename

Expand Down