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

rasterio.open typing updates #11

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

alecglen
Copy link

Offering two improvements to the open() type stub:

  1. The DatasetReader and DatasetWriter stubs should support context manager ("with _ as _") syntax, achieved through the __enter__ and __exit__ methods.

    image
  2. Union[DatasetReader, DatasetWriter] is not quite the correct return type because it restricts usage to only methods contained in both classes. Instead, I overloaded the stub to return either a DatasetReader or DatasetWriter conditionally based on the mode parameter. This also results in more accurate type hinting.

    image

@@ -30,7 +46,7 @@ def open(
nodata: Nodata | None = ...,
sharing: bool = ...,
**kwargs
) -> Union[DatasetReader, DatasetWriter]: ...
) -> DatasetReader: ...
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does this work? Usually you need an overload for each discriminator, and then a fallback, but maybe it works a little differently in stub files.

@overload
def open(
    fp,
    mode: str = "w",
    ...
    ) -> DatasetWriter: ...

@overload
def open(
    fp,
    mode: str = "r",
    ...
    ) -> DatasetReader: ...

def open(
    fp,
    mode: str = ...,
	...
    ) -> Union[DatasetReader, DatasetWriter]: ...

For example, does rasterio support anything other than w and r? Does it support an append mode?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good question, I was just getting it going for my use case and then figured I'd share the change. I'll take a look and update it to be more comprehensive.

Comment on lines +29 to +30
def __enter__(self, *args, **kwargs) -> DatasetReader: ...
def __exit__(self, *args, **kwargs): ...
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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