Skip to content
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

A fileseq with no padding is still able to be iterated through #127

Closed
walrusVision opened this issue Nov 5, 2023 · 4 comments
Closed
Labels
v2 Major version 2

Comments

@walrusVision
Copy link
Contributor

A FileSequence with no padding is able to be iterated through, but with a result which is non-obvious. The range gets combined into the file name and a single file is returned

Current behavior

>>> fs = fileseq.FileSequence("foo.exr")
>>> list(fs)
['foo.exr']
>>> fs.setFrameRange("1-10")
>>> list(fs)
['foo1-10.exr'] 

Setting a padding explicitly allows it to work as expected.

>>> fs.padding()
''
>>> fs.setPadding("#")
>>> list(fs)
['foo0001.exr', 'foo0002.exr', 'foo0003.exr', 'foo0004.exr', 'foo0005.exr', 'foo0006.exr', 'foo0007.exr', 'foo0008.exr', 'foo0009.exr', 'foo0010.exr']

For the expected behavior I'd expect it to either raise an error, or fallback to some default padding.

For example

>>> fs = fileseq.FileSequence("foo.exr")
>>> fs.setFrameRange("1-10")
>>> list(fs)
['foo1.exr', 'foo2.exr', 'foo3.exr', 'foo4.exr', 'foo5.exr', 'foo6.exr', 'foo7.exr', 'foo8.exr', 'foo9.exr', 'foo10.exr']

OR

>>> fs = fileseq.FileSequence("foo.exr")
>>> fs.setFrameRange("1-10")
>>> list(fs)
# Some exception gets thrown here
@justinfx justinfx added the v2 Major version 2 label Nov 5, 2023
@justinfx
Copy link
Owner

justinfx commented Nov 5, 2023

Thanks for registering the issue. I wouldn't expect it to raise an error since the class acts as a valid container and should provide iteration regardless of the input. Defaulting to a padding char when setting a range would be a nice idea, and I think since it is a potentially breaking behaviour, we should expect it to be a v2 feature.

@justinfx
Copy link
Owner

There is now a v2 branch available, if this change wants to be a PR

@justinfx
Copy link
Owner

@walrusVision do you want to have a look at #130 ?
I have opted to set a default pad char.

@justinfx
Copy link
Owner

Merged to v2 branch

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
v2 Major version 2
Projects
None yet
Development

No branches or pull requests

2 participants