Add support for removing version AND (not or) framenumber. #68
Conversation
The existing regex patterns only handle either removing the version number from a non-sequence path or the frame-number from a sequence path, not both together (removing the version and converting the frame number to padding). This update adds a new regex pattern that should(I'm a regex novice) allow removal of version number from a sequence and conversion of framenumber to padding.
There was a problem hiding this comment.
Hey @reformstudios! Thanks for the PR! I definitely see the need for this. I've added some comments for you. Let me know what you think. Thanks!!
| @@ -1,3 +1,7 @@ | |||
| ''' | |||
There was a problem hiding this comment.
guessing this was left by accident?
| @@ -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 '.####' | |||
There was a problem hiding this comment.
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. |
|
|
||
| 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): |
There was a problem hiding this comment.
suggest making the new arg strip_frame=False or something like that.
There was a problem hiding this comment.
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!
There was a problem hiding this comment.
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.
| @@ -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) | |||
There was a problem hiding this comment.
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...There was a problem hiding this comment.
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.
| @@ -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: | |||
|
Sounds good.
…On 11 April 2018 at 15:49, Josh Tomlinson ***@***.***> wrote:
***@***.**** commented on this pull request.
------------------------------
In hooks/path_info.py
<#68 (comment)>
:
>
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):
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.
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#68 (comment)>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/AEfLwNg-nBtD_YvTm_3LQd0nR0absXHFks5tnhgDgaJpZM4TPwYF>
.
|
The existing regex patterns only handle either removing the version number from a non-sequence path or the frame-number from a sequence path, not both together (removing the version and converting the frame number to padding).
This update adds a new regex pattern that should(I'm a regex novice) allow removal of version number from a sequence and conversion of framenumber to padding.