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

SNOW-801097: Pyre type checker returns unexpected type errors with snowpark-python #810

Closed
nerdinand opened this issue Apr 25, 2023 · 3 comments
Labels
bug Something isn't working triaged

Comments

@nerdinand
Copy link

nerdinand commented Apr 25, 2023

Please answer these questions before submitting your issue. Thanks!

  1. What version of Python are you using?
Python 3.8.16 (default, Mar  8 2023, 23:07:01) 
[Clang 14.0.0 (clang-1400.0.29.202)]
  1. What operating system and processor architecture are you using?
macOS-13.3.1-arm64-arm-64bit
  1. What are the component versions in the environment (pip freeze)?

Redacted to the most relevant packages:

pyre-check==0.9.18
pyre-extensions==0.0.30
snowflake-connector-python==3.0.2
snowflake-snowpark-python==1.1.0
  1. What did you do?

See: facebook/pyre-check#716

We're using snowpark-python in conjunction with the pyre type checker. While doing this we came across some weird type errors, e.g. with this piece of code:

import snowflake.snowpark.functions as spf

spf.min("foo")

We get the type error:

src/reproduction.py:3:8 Incompatible parameter type [6]: In call `spf.min`, for 1st positional argument, expected `ColumnOrName` but got `str`.

Since ColumnOrName is defined as

ColumnOrName = NewType("ColumnOrName", Union["snowflake.snowpark.column.Column", str])

we would not expect such an error.

As outlined in facebook/pyre-check#716 (comment), the code

from typing import NewType, Union

ColumnOrName = NewType("ColumnOrName", Union["snowflake.snowpark.column.Column", str])

def min(e: ColumnOrName) -> None:
    pass

min("foo")

produces to two type errors:

ƛ Found 2 type errors!
src/reproduction.py:3:39 Invalid inheritance [39]: `Union[snowflake.snowpark.column.Column, str]` is not a valid parent class.
src/reproduction.py:8:4 Incompatible parameter type [6]: In call `min`, for 1st positional argument, expected `ColumnOrName` but got `str`.

This leads to our assumption that snowpark-python isn't using NewType correctly. Indeed, the same snippet without NewType:

from typing import NewType, Union

ColumnOrName = Union["snowflake.snowpark.column.Column", str]

def min(e: ColumnOrName) -> None:
    pass

min("foo")

passes the type checks.

  1. What did you expect to see?

This piece of code should not result in an error with the pyre type checker:

import snowflake.snowpark.functions as spf

spf.min("foo")
@nerdinand nerdinand added bug Something isn't working needs triage Initial RCA is required labels Apr 25, 2023
@sfc-gh-mkeller sfc-gh-mkeller changed the title Pyre type checker returns unexpected type errors with snowpark-python SNOW-801097: Pyre type checker returns unexpected type errors with snowpark-python Apr 25, 2023
@sfc-gh-mkeller
Copy link
Collaborator

I agree, according to the docs NewType is meant for distinctive types which this shouldn't be.
Great investigation @nerdinand , would you like to raise a PR to fix this issue?

@sfc-gh-mkeller sfc-gh-mkeller added triaged and removed needs triage Initial RCA is required labels Apr 25, 2023
@sfc-gh-mkeller
Copy link
Collaborator

sfc-gh-mkeller commented Apr 25, 2023

Actually, this is should be fixed by #799

@alexmojaki
Copy link
Contributor

This can be closed now, right?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working triaged
Projects
None yet
Development

No branches or pull requests

3 participants