Type stubs

[johhnry]

Hi,

Are there any plans to provide type stubs for the library? (.pyi files not type hinting the code itself)

With the progress of type checkers such as MyPy, it would be nice to use it with Fileseq. See: https://typing.readthedocs.io/en/latest/source/stubs.html

If nobody is working on it I might do a merge request!

[justinfx]

I can't say there has been any plan to create stubs at this point. Is there a specific benefit to generating and maintaining external type stubs for a pure python library? Would it not be easier to maintain if they were inline and used the comment approach as opposed to the py3 approach? We could also just hold off and make this a feature that uses proper type hints when we cut off the py2 support entirely.

What are your thoughts on the need for external stubs for a pure python library, vs the cost of maintaining them to make sure they stay aligned?

[johhnry]

Since the library targets Python 2.7 as well as Python 3.x providing type stubs is a benefit since you can't directly add type hints in the code.

Using comments is a legacy way to type a library as described here: https://realpython.com/lessons/type-comments/

Type comments are more verbose and might conflict with other kinds of comments in your code like linter directives. However, they can be used in code bases that don’t support annotation

The best solution is of course to drop Python 2.7 support and fully type the library with Python 3 type hints without having to maintain .pyi stubs.

[justinfx]

I would say that for a pure python library (vs a compiled extension), maintaining external type hints feels excessive. So I am inclined to go with either the legacy type comments (which I have used successfully in other py2/3 projects) or as part a new major version release that drops py2 entirely.
We should probably drop py2 and only do bug fixes for the v1 version stream (my own studio is still migrating to py3).

[justinfx]

Drop py2 support: #119

[johhnry]

Hi again,

For a future addition to the library I am reporting that I am getting the following error with MyPy when importing fileseq:

# main.py
import fileseq

main.py:1: error: Skipping analyzing "fileseq": module is installed, but missing library stubs or py.typed marker  [import-untyped]
main.py:1: note: See https://mypy.readthedocs.io/en/stable/running_mypy.html#missing-imports
Found 1 error in 1 file (checked 1 source file)

This is the PEP section about the py.typed file:
https://peps.python.org/pep-0561/#packaging-type-information

[justinfx]

I can work on dropping py2 support this weekend. And we can add the marker file.

[johhnry]

@justinfx So cool, thanks for taking time on this! If you need any help let me know

[justinfx]

@johhnry do you want to review #129 and propose any changes?

[johhnry]

@justinfx thank you this is much much better! I don't know which type checker you use, but if I check with MyPy, I get:

❯ mypy src         
src/fileseq/frameset.py:489: error: Need type annotation for "result" (hint: "result: List[<type>] = ...")  [var-annotated]
src/fileseq/frameset.py:921: error: Variable "builtins.NotImplemented" is not valid as a type  [valid-type]
src/fileseq/frameset.py:921: note: See https://mypy.readthedocs.io/en/stable/common_issues.html#variables-vs-type-aliases
src/fileseq/frameset.py:938: error: Variable "builtins.NotImplemented" is not valid as a type  [valid-type]
src/fileseq/frameset.py:938: note: See https://mypy.readthedocs.io/en/stable/common_issues.html#variables-vs-type-aliases
src/fileseq/frameset.py:955: error: Variable "builtins.NotImplemented" is not valid as a type  [valid-type]
src/fileseq/frameset.py:955: note: See https://mypy.readthedocs.io/en/stable/common_issues.html#variables-vs-type-aliases
src/fileseq/frameset.py:1231: error: Incompatible types in assignment (expression has type "DecimalTuple", variable has type "Decimal")  [assignment]
src/fileseq/frameset.py:1232: error: "Decimal" has no attribute "digits"  [attr-defined]
src/fileseq/frameset.py:1232: error: "Decimal" has no attribute "exponent"  [attr-defined]
src/fileseq/frameset.py:1237: error: Unsupported operand types for / ("Decimal" and "None")  [operator]
src/fileseq/frameset.py:1237: note: Right operand is of type "Decimal | None"
src/fileseq/frameset.py:1239: error: Argument 1 to "scaleb" of "Decimal" has incompatible type "Literal['n', 'N', 'F'] | int"; expected "Decimal | int"  [arg-type]
src/fileseq/frameset.py:1243: error: Argument 1 to "_build_frange_part" of "FrameSet" has incompatible type "Decimal"; expected "int"  [arg-type]
src/fileseq/frameset.py:1243: error: Argument 2 to "_build_frange_part" of "FrameSet" has incompatible type "Decimal"; expected "int | None"  [arg-type]
src/fileseq/frameset.py:1246: error: "Generator" expects 3 type arguments, but 1 given  [type-arg]
src/fileseq/frameset.py:1358: error: Argument 1 has incompatible type "Any | None"; expected "int"  [arg-type]
src/fileseq/frameset.py:1366: error: Argument 1 has incompatible type "Any | None"; expected "int"  [arg-type]
src/fileseq/filesequence.py:68: error: Need type annotation for "_frame_pad"  [var-annotated]
src/fileseq/filesequence.py:68: error: Need type annotation for "_subframe_pad"  [var-annotated]
src/fileseq/filesequence.py:572: error: Incompatible types in assignment (expression has type "int | Decimal | str", variable has type "str | None")  [assignment]
src/fileseq/filesequence.py:576: error: Argument 1 to "join" of "str" has incompatible type "tuple[str, str, str | None, str | Any]"; expected "Iterable[str]"  [arg-type]
src/fileseq/filesequence.py:771: error: Incompatible default for argument "using" (default has type "None", argument has type "FileSequence")  [assignment]
src/fileseq/filesequence.py:771: note: PEP 484 prohibits implicit Optional. Accordingly, mypy has changed its default to no_implicit_optional=True
src/fileseq/filesequence.py:771: note: Use https://github.com/hauntsaninja/no_implicit_optional to automatically upgrade your codebase
src/fileseq/filesequence.py:773: error: "Generator" expects 3 type arguments, but 1 given  [type-arg]
src/fileseq/filesequence.py:806: error: Need type annotation for "seqs" (hint: "seqs: Dict[<type>, <type>] = ...")  [var-annotated]
src/fileseq/filesequence.py:818: error: Need type annotation for "frames" (hint: "frames: Set[<type>] = ...")  [var-annotated]
src/fileseq/filesequence.py:820: error: Need type annotation for "path"  [var-annotated]
src/fileseq/filesequence.py:907: error: Need type annotation for "current_frames" (hint: "current_frames: List[<type>] = ...")  [var-annotated]
src/fileseq/filesequence.py:1039: error: Item "None" of "Match[str] | None" has no attribute "group"  [union-attr]
src/fileseq/filesequence.py:1055: error: Incompatible types in assignment (expression has type "filter[str]", variable has type "list[str]")  [assignment]
src/fileseq/filesequence.py:1059: error: Incompatible types in assignment (expression has type "filter[str]", variable has type "list[str]")  [assignment]
Found 27 errors in 2 files (checked 7 source files)

Then it depends if you want to type the whole library, including every function arguments and return types, what do you think?

[justinfx]

I don't use MyPy. Just the standard checker/linter integrated into Jetbrains. I've never used MyPy so I am not sure if it is far more aggressive. If enabling this py.typed feature is going to require adhering to MyPy standards then I'm probably going to remove it, as it isn't worth the effort right now. Otherwise I would need to add MyPy to the CI actions to ensure the code always passes the type checker.

Should we just remove the py.typed marker?

[justinfx]

I've just pushed another commit that addresses all the errors reported by MyPy: 86b078a

Does that look right to you at this point? I would need to add a mypy check to the CI as well.

[johhnry]

@justinfx Adding the py.typed doesn't force you to use MyPy standards, it's just a way to tell type checkers and linters that your library provides its own type hints. We shouldn't remove it!

For example in VSCode I use Pylance + Pyright for the type checking in strict mode which works well. On the other hand, MyPy is open-source and is widely used in well established Python libraries.

As for your last commit, it works like a charm! MyPy found no issues in the source files which is great. You can add MyPy as a CI check as well but it might require a little bit more setup if you have time for that 👍

[justinfx]

I've added mypy type checking to the CI and merged to the v2 branch, which is being prepared to drop python2 support:
https://github.com/justinfx/fileseq/actions/runs/7892283794

[johhnry]

@justinfx awesome, thank you!

[johhnry]

Hi @justinfx do you have an idea when this will be released? Are you waiting for other bug fixes to be solved for the v2?

[justinfx]

@johhnry i was waiting on one more v2 issue but I'm likely going to close it as a won't-fix. Might be able to release v2 within a day or two

[justinfx]

@johhnry I've released v2.0.0

[johhnry]

@justinfx thanks for the quick release!

One thing I noticed is that I get this error with Mypy (and Pylance in VSCode):

# main.py
from fileseq import FileSequence

seq = FileSequence("test")

❯ mypy --no-implicit-reexport ./main.py
main.py:1: error: Module "fileseq" does not explicitly export attribute "FileSequence"  [attr-defined]
Found 1 error in 1 file (checked 1 source file)

An easy fix is to do from fileseq.filesequence import FileSequence but it's less concise.
I used the --no-implicit-reexport option which is enabled when using mypy --strict and is commonly found in Mypy configs.

For example there's an issue on Dacite about this: konradhalas/dacite#133

---

As for the type hints themselves, your Mypy configuration was a bit too loose and the following functions that I am using don't have a proper type signature:

• FileSequence.setFrameRange (should be def setFrameRange(self, frange: FrameSet) -> None
• FrameSet.issuperset (should be def issuperset(self, other: object) -> bool | NotImplemented)
• FrameSet.difference (this one is a little bit tricky because it can be objects that can be cast to FrameSet but it can simply be object I guess like def difference(self, *other: object) -> FrameSet:)

A more strict Mypy configuration is explained in details here: https://justincaustin.com/blog/mypy-tips-and-tricks/

What do you think?

[justinfx]

@johhnry please submit a PR. Like I said, I don't use MyPy, and I kind of did this on request and tried to check it with your setup.
Happy to merge improvements and do a patch release.

[johhnry]

@justinfx I understand, I'll do a PR then. Thanks again for your time!

[dkbarn]

Was that PR ever made? I am still getting the following mypy errors in fileseq:

error: Module "fileseq" does not explicitly export attribute "FileSequence"  [attr-defined]
    from fileseq import FileSequence, FrameSet, ParseException
    ^
error: Module "fileseq" does not explicitly export attribute "FrameSet"  [attr-defined]
    from fileseq import FileSequence, FrameSet, ParseException
    ^
error: Module "fileseq" does not explicitly export attribute "ParseException"  [attr-defined]
    from fileseq import FileSequence, FrameSet, ParseException
    ^

[justinfx]

@dkbarn no the PR hasn't been submitted as of yet

[justinfx]

@dkbarn as part of another fix, I have updated the type annotations to pass the strict mypy check