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

Pyright enable standard type checking mode #1084

Merged
merged 6 commits into from
Jan 27, 2025

Conversation

vlaci
Copy link
Contributor

@vlaci vlaci commented Jan 21, 2025

See individual commits for explanations

@vlaci vlaci force-pushed the pyright-standard-type-checking-mode branch from a63767a to cf1bf3c Compare January 21, 2025 16:15
unblob/models.py Outdated Show resolved Hide resolved
unblob/handlers/compression/_gzip_reader.py Show resolved Hide resolved
@qkaiser
Copy link
Contributor

qkaiser commented Jan 24, 2025

Put my takes in threads started by @e3krisztian. Generally agree with the changes.

@vlaci not to be pedantic but you should fix the typo in your commit messages (This check is currently turned of -> This check is currently turned off).

@qkaiser qkaiser added enhancement New feature or request python Pull requests that update Python code labels Jan 24, 2025
@vlaci vlaci force-pushed the pyright-standard-type-checking-mode branch 2 times, most recently from 465c024 to ac68312 Compare January 27, 2025 11:52
vlaci added 6 commits January 27, 2025 16:20
This check is currently turned off in our `basic` `typeCheckingMode`
profile, but is enabled in the `standard` profile.

Things done here:

- matched the signature of `__exit__` and `seekable` to `mmap`'s type
  stub.
- indicate that `seekable` is present in `mmap` in Python 3.13+
- ignored the `seek` method, as we explicitly wanted it to return the
  file position similarly to the standard file api

Error:

    unblob/file_utils.py
      unblob/file_utils.py:70:9 - error: Method "seek" overrides class "mmap" in an incompatible manner
        Return type mismatch: base method returns type "None", override returns type "int"
          "int" is not assignable to "None" (reportIncompatibleMethodOverride)
      unblob/file_utils.py:94:9 - error: Method "__exit__" overrides class "mmap" in an incompatible manner
        Positional parameter count mismatch; base method has 2, but override has 4
        Parameter "args" is missing in override (reportIncompatibleMethodOverride)
      unblob/file_utils.py:103:9 - error: Method "seekable" overrides class "mmap" in an incompatible manner
        Return type mismatch: base method returns type "Literal[True]", override returns type "bool"
          "bool" is not assignable to type "Literal[True]" (reportIncompatibleMethodOverride)
This check is currently turned off in our `basic` `typeCheckingMode`
profile, but is enabled in the `standard` profile.

Things done here:

- matched the signature of `default` with `JSONEncoder.default`

Error:

    unblob/models.py
      unblob/models.py:258:9 - error: Method "default" overrides class "JSONEncoder" in an incompatible manner
        Parameter 2 name mismatch: base parameter is named "o", override parameter is named "obj" (reportIncompatibleMethodOverride)
This check is currently turned off in our `basic` `typeCheckingMode`
profile, but is enabled in the `standard` profile.

Things done here:

- removed the invalid cross inheritance, as the bases conflict on
  `extract` method

                 Extractor -> Command
                     ^            |
                     x            |
                     v            v
        DirectoryExtractor -> MultiFileCommand

Error:

    unblob/extractors/command.py
      unblob/extractors/command.py:106:9 - error: Method "extract" overrides class "Command" in an incompatible manner
        Parameter 2 name mismatch: base parameter is named "inpath", override parameter is named "paths" (reportIncompatibleMethodOverride)
This check is currently turned off in our `basic` `typeCheckingMode`
profile, but is enabled in the `standard` profile.

Things done here:

- made the type of `Handler.EXTRACTOR` covariant, so child-classes can
  further narrow its type

Error:

    unblob/handlers/archive/cpio.py
      unblob/handlers/archive/cpio.py:413:5 - error: "EXTRACTOR" overrides symbol of same name in class "Handler"
        Variable is mutable so its type is invariant
          Override type "_CPIOExtractorBase" is not the same as base type "Extractor | None" (reportIncompatibleVariableOverride)
This check is currently turned off in our `basic` `typeCheckingMode`
profile, but is enabled in the `standard` profile.

Things done here:

- instead of incompatible overriding `read`, added a new `read_all`
  method, to better express the functionality of that method

Error:

    unblob/handlers/compression/_gzip_reader.py
      unblob/handlers/compression/_gzip_reader.py:18:9 - error: Method "read" overrides class "DecompressReader" in an incompatible manner
        Positional parameter count mismatch; base method has 2, but override has 1
        Parameter 2 mismatch: base parameter "size" is keyword parameter, override parameter is position-only (reportIncompatibleMethodOverride)
@vlaci vlaci force-pushed the pyright-standard-type-checking-mode branch from ac68312 to a354a12 Compare January 27, 2025 15:21
@vlaci vlaci enabled auto-merge January 27, 2025 15:21
@vlaci vlaci added this pull request to the merge queue Jan 27, 2025
Merged via the queue into main with commit ea8d377 Jan 27, 2025
14 checks passed
@vlaci vlaci deleted the pyright-standard-type-checking-mode branch January 27, 2025 15:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request python Pull requests that update Python code
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants