FileSequence.basename cut at the wrong place with still frames

[mathbou]

I use findSequencesOnDisk to retrieve my sequences, it also returns the files without padding as I want it. But with the still frames, the basename methods cut the name at the first point, not the last.

I have this kind of filename: PROJECT_010_110_CAT.RGB_color.exr
The basename method returns: PROJECT_010_110_CAT.
The extension method returns: .RGB_color.exr

With a filename with padding: PROJECT_010_110_CAT.RGB_color.0042.exr
The basename returned is: PROJECT_010_110_CAT.RGB_color.
And the extension is: .exr

[justinfx]

I'm guessing this problem was introduced via the fix for #56. It was meant to support extensions with multiple dot components. But it would seem it is not properly ignoring patterns where there is no frame range. It shouldn't parse the longer extension in this case. I will try and look into this shortly.

[justinfx]

I've pushed a 65_long_ext_parsing branch with a fix for this problem. However I am a bit torn on some resulting behaviour:

Given input: file.v2.tar.gz

Currently the output will be: basename: file.v2 ; ext: .tar.gz
There is logic that detects when the split was on a possible version number instead of a frame, and it merged it into the basename, leaving the rest as the extension. I am torn on whether the output should actually be: basename: file.v2.tar ; ext: .gz

Will need to think about this.

[yawpitch]

Hmm, at first blush I like the ".tar.gz", but something tells me it's probably better to try and remain consistent with os.path.splitext and pathlib.Path.suffix.

…

On Jun 17, 2018, 06:22 +0100, Justin Israel ***@***.***>, wrote: I've pushed a 65_long_ext_parsing branch with a fix for this problem. However I am a bit torn on some resulting behaviour: Given input: file.v2.tar.gz Currently the output will be: basename: file.v2 ; ext: .tar.gz There is logic that detects when the split was on a possible version number instead of a frame, and it merged it into the basename, leaving the rest as the extension. I am torn on whether the output should actually be: basename: file.v2.tar ; ext: .gz Will need to think about this. — You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub, or mute the thread.

[justinfx]

Yea. I thought it would be good to try and preserve things like that, considering the use case. The problem is that if I don't capture it, where do I store the extra string component without dropping it? file.0100.tar.gz If the extension is captured as .gz, then .tar is lost. It isn't as simple as splitext since we store dirname, basename, range, pad, ext On Wed, Jun 20, 2018, 8:37 AM Michael Morehouse <notifications@github.com> wrote:

…

Hmm, at first blush I like the ".tar.gz", but something tells me it's probably better to try and remain consistent with os.path.splitext and pathlib.Path.suffix. On Jun 17, 2018, 06:22 +0100, Justin Israel ***@***.***>, wrote: > I've pushed a 65_long_ext_parsing branch with a fix for this problem. However I am a bit torn on some resulting behaviour: > Given input: file.v2.tar.gz > Currently the output will be: basename: file.v2 ; ext: .tar.gz > There is logic that detects when the split was on a possible version number instead of a frame, and it merged it into the basename, leaving the rest as the extension. I am torn on whether the output should actually be: basename: file.v2.tar ; ext: .gz > Will need to think about this. > — > You are receiving this because you are subscribed to this thread. > Reply to this email directly, view it on GitHub, or mute the thread. — You are receiving this because you were assigned. Reply to this email directly, view it on GitHub <#65 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AARPxuSYCGZYnLyIwR3zJigqPH51vV__ks5t-WEVgaJpZM4UiUfR> .

[yawpitch]

Yeah, not sure. This is where pathlib.Path offers the .suffixes method, which gives you a list of all the extension components... I've wondered about rewriting FileSeq in light of pathlib, subclassing Path to incorporate the idea of frame and padding, but leaving the non-sequential extension components in .suffixes, but I was holding off until having time to rewrite for 3 as well.

…

On Jun 19, 2018, 22:28 +0100, Justin Israel ***@***.***>, wrote: Yea. I thought it would be good to try and preserve things like that, considering the use case. The problem is that if I don't capture it, where do I store the extra string component without dropping it? file.0100.tar.gz If the extension is captured as .gz, then .tar is lost. It isn't as simple as splitext since we store dirname, basename, range, pad, ext On Wed, Jun 20, 2018, 8:37 AM Michael Morehouse ***@***.***> wrote: > Hmm, at first blush I like the ".tar.gz", but something tells me it's > probably better to try and remain consistent with os.path.splitext and > pathlib.Path.suffix. > On Jun 17, 2018, 06:22 +0100, Justin Israel ***@***.***>, > wrote: > > I've pushed a 65_long_ext_parsing branch with a fix for this problem. > However I am a bit torn on some resulting behaviour: > > Given input: file.v2.tar.gz > > Currently the output will be: basename: file.v2 ; ext: .tar.gz > > There is logic that detects when the split was on a possible version > number instead of a frame, and it merged it into the basename, leaving the > rest as the extension. I am torn on whether the output should actually be: > basename: file.v2.tar ; ext: .gz > > Will need to think about this. > > — > > You are receiving this because you are subscribed to this thread. > > Reply to this email directly, view it on GitHub, or mute the thread. > > — > You are receiving this because you were assigned. > > > Reply to this email directly, view it on GitHub > <#65 (comment)>, or mute > the thread > <https://github.com/notifications/unsubscribe-auth/AARPxuSYCGZYnLyIwR3zJigqPH51vV__ks5t-WEVgaJpZM4UiUfR> > . > — You are receiving this because you commented. Reply to this email directly, view it on GitHub, or mute the thread.

[justinfx]

Well I was also thinking of rewriting fileseq to be a binding of libfileseq (c++) so that I don't have to maintain the 3 individual language ports that I currently look after. Also the gofileseq/libfileseq versions have a more memory efficient solution to storing range data than the python implementation. On Wed, Jun 20, 2018, 11:41 AM Michael Morehouse <notifications@github.com> wrote:

…

Yeah, not sure. This is where pathlib.Path offers the .suffixes method, which gives you a list of all the extension components... I've wondered about rewriting FileSeq in light of pathlib, subclassing Path to incorporate the idea of frame and padding, but leaving the non-sequential extension components in .suffixes, but I was holding off until having time to rewrite for 3 as well. On Jun 19, 2018, 22:28 +0100, Justin Israel ***@***.***>, wrote: > Yea. I thought it would be good to try and preserve things like that, > considering the use case. The problem is that if I don't capture it, where > do I store the extra string component without dropping it? > > > file.0100.tar.gz > > If the extension is captured as .gz, then .tar is lost. It isn't as simple > as splitext since we store dirname, basename, range, pad, ext > > On Wed, Jun 20, 2018, 8:37 AM Michael Morehouse < ***@***.***> > wrote: > > > Hmm, at first blush I like the ".tar.gz", but something tells me it's > > probably better to try and remain consistent with os.path.splitext and > > pathlib.Path.suffix. > > On Jun 17, 2018, 06:22 +0100, Justin Israel ***@***.*** >, > > wrote: > > > I've pushed a 65_long_ext_parsing branch with a fix for this problem. > > However I am a bit torn on some resulting behaviour: > > > Given input: file.v2.tar.gz > > > Currently the output will be: basename: file.v2 ; ext: .tar.gz > > > There is logic that detects when the split was on a possible version > > number instead of a frame, and it merged it into the basename, leaving the > > rest as the extension. I am torn on whether the output should actually be: > > basename: file.v2.tar ; ext: .gz > > > Will need to think about this. > > > — > > > You are receiving this because you are subscribed to this thread. > > > Reply to this email directly, view it on GitHub, or mute the thread. > > > > — > > You are receiving this because you were assigned. > > > > > > Reply to this email directly, view it on GitHub > > <#65 (comment)>, or mute > > the thread > > < https://github.com/notifications/unsubscribe-auth/AARPxuSYCGZYnLyIwR3zJigqPH51vV__ks5t-WEVgaJpZM4UiUfR > > > . > > > — > You are receiving this because you commented. > Reply to this email directly, view it on GitHub, or mute the thread. — You are receiving this because you were assigned. Reply to this email directly, view it on GitHub <#65 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AARPxgWm7N5-NXBGdiZLJZrdP99EvQe5ks5t-YwogaJpZM4UiUfR> .

[yawpitch]

Understandable, though that doesn't directly impact whether or not you'd want to have the Python version using pathlib.Path semantics to provide a more useful (and Pythonic) interface to that binding.

…

On Jun 20, 2018, 02:31 +0100, Justin Israel ***@***.***>, wrote: Well I was also thinking of rewriting fileseq to be a binding of libfileseq (c++) so that I don't have to maintain the 3 individual language ports that I currently look after. Also the gofileseq/libfileseq versions have a more memory efficient solution to storing range data than the python implementation. On Wed, Jun 20, 2018, 11:41 AM Michael Morehouse ***@***.***> wrote: > Yeah, not sure. This is where pathlib.Path offers the .suffixes method, > which gives you a list of all the extension components... I've wondered > about rewriting FileSeq in light of pathlib, subclassing Path to > incorporate the idea of frame and padding, but leaving the non-sequential > extension components in .suffixes, but I was holding off until having time > to rewrite for 3 as well. > On Jun 19, 2018, 22:28 +0100, Justin Israel ***@***.***>, > wrote: > > Yea. I thought it would be good to try and preserve things like that, > > considering the use case. The problem is that if I don't capture it, > where > > do I store the extra string component without dropping it? > > > > > > file.0100.tar.gz > > > > If the extension is captured as .gz, then .tar is lost. It isn't as > simple > > as splitext since we store dirname, basename, range, pad, ext > > > > On Wed, Jun 20, 2018, 8:37 AM Michael Morehouse < > ***@***.***> > > wrote: > > > > > Hmm, at first blush I like the ".tar.gz", but something tells me it's > > > probably better to try and remain consistent with os.path.splitext and > > > pathlib.Path.suffix. > > > On Jun 17, 2018, 06:22 +0100, Justin Israel ***@***.*** > >, > > > wrote: > > > > I've pushed a 65_long_ext_parsing branch with a fix for this problem. > > > However I am a bit torn on some resulting behaviour: > > > > Given input: file.v2.tar.gz > > > > Currently the output will be: basename: file.v2 ; ext: .tar.gz > > > > There is logic that detects when the split was on a possible version > > > number instead of a frame, and it merged it into the basename, leaving > the > > > rest as the extension. I am torn on whether the output should actually > be: > > > basename: file.v2.tar ; ext: .gz > > > > Will need to think about this. > > > > — > > > > You are receiving this because you are subscribed to this thread. > > > > Reply to this email directly, view it on GitHub, or mute the thread. > > > > > > — > > > You are receiving this because you were assigned. > > > > > > > > > Reply to this email directly, view it on GitHub > > > <#65 (comment)>, > or mute > > > the thread > > > < > https://github.com/notifications/unsubscribe-auth/AARPxuSYCGZYnLyIwR3zJigqPH51vV__ks5t-WEVgaJpZM4UiUfR > > > > > . > > > > > — > > You are receiving this because you commented. > > Reply to this email directly, view it on GitHub, or mute the thread. > > — > You are receiving this because you were assigned. > > > Reply to this email directly, view it on GitHub > <#65 (comment)>, or mute > the thread > <https://github.com/notifications/unsubscribe-auth/AARPxgWm7N5-NXBGdiZLJZrdP99EvQe5ks5t-YwogaJpZM4UiUfR> > . > — You are receiving this because you commented. Reply to this email directly, view it on GitHub, or mute the thread.

[justinfx]

It looks like the ANTLR4 grammar now used in Go/C++ would solve this original issue. We have proposed migrating python fileseq to the grammar as well: #147

I think that leaves pathlib.Path as more of a cosmetic/UX benefit, since the grammar directly handles a lot of logic. I think we could still consider supporting accepting the type in the FileSequence constructor. Is there a strong case for returning it instead of str in some places?

[justinfx]

The new v3.0.0 has been merged and tagged. However it didn't end up addressing this particular situation from the grammar parser either. It is really the same problem as with the regex, where you could say the rule should be applied only to plain file paths that do not have frame ranges, but you have to accept that something like ".tar.gz" will no longer be the outcome.

FWIW, this is where we maintain the current 3 special case post processing rules, and it would need to become a 4th rule:
https://github.com/justinfx/fileseq/blob/v3.0.0/src/fileseq/parser/parse_postprocess.py#L8

As for the idea of basing FileSequence on top of pathlib.Path, I think ultimately we don't gain much. pathlib still just uses dot-splitting and returning the last component for ext. Similar to how it can return all dot-separated components if you want to know all the "suffixes". However it doesn't really have the extra knowledge that is needed when we are parsing file sequence patterns that have to consider frame padding positions. I think we are better off sticking with what we have for the base class.