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

Make CloudPath.is_valid_cloudpath a TypeGuard #337

Conversation

ringohoffman
Copy link
Contributor

This is actually a TypeGuard I had implemented in my own codebase, but I recently realized it is doing exactly the same thing as is_valid_cloudpath. This allows us to infer that CloudPaths are the same type of object as the class is_valid_cloudpath is called on!

import cloudpathlib


def get_cloudpath_str() -> str:
    return "s3://other/path"


def get_cloudpath() -> cloudpathlib.CloudPath:
    return cloudpathlib.CloudPath("s3://other/path")


other_path_str = get_cloudpath_str()
if cloudpathlib.S3Path.is_valid_cloudpath(other_path_str):
    reveal_type(other_path_str)
    # no change from before:
    # pyright : Type of "other_path_str" is "str"
    # mypy : Revealed type is "builtins.str"

other_path = get_cloudpath()
if cloudpathlib.S3Path.is_valid_cloudpath(other_path):
    reveal_type(other_path)
    # before:
    # pyright : Type of "other_path" is "CloudPath"
    # mypy : Revealed type is "cloudpathlib.cloudpath.CloudPath"

    # this PR:
    # pyright : Type of "other_path" is "S3Path"
    # mypy : Revealed type is "Self`2"

I opened an issue with mypy regarding this strange behavior which only seems to arise on TypeGuard methods: python/mypy#15401

This allows us to infer that CloudPaths are the same type of object as the class is_valid_cloudpath is called on
@codecov
Copy link

codecov bot commented Jun 9, 2023

Codecov Report

Merging #337 (fe48641) into master (4211d49) will decrease coverage by 0.4%.
The diff coverage is 83.3%.

Additional details and impacted files
@@           Coverage Diff            @@
##           master    #337     +/-   ##
========================================
- Coverage    94.2%   93.8%   -0.4%     
========================================
  Files          22      22             
  Lines        1473    1484     +11     
========================================
+ Hits         1388    1393      +5     
- Misses         85      91      +6     
Impacted Files Coverage Δ
cloudpathlib/cloudpath.py 92.2% <83.3%> (-0.3%) ⬇️

... and 2 files with indirect coverage changes

@pjbull pjbull changed the base branch from master to 337-local-branch June 12, 2023 17:23
@pjbull pjbull merged commit e8ac247 into drivendataorg:337-local-branch Jun 12, 2023
22 of 24 checks passed
pjbull added a commit that referenced this pull request Jun 12, 2023
This allows us to infer that CloudPaths are the same type of object as the class is_valid_cloudpath is called on

Co-authored-by: Matthew Hoffman <matthew@protopia.ai>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants