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-1417060: Pylance throws errors when using copy_options with copy_into_location method #1574

Closed
Yoshi-Egawa opened this issue May 14, 2024 · 6 comments
Assignees
Labels
enhancement The issue is a request for improvement or a new feature status-triage_done Initial triage done, will be further handled by the driver team

Comments

@Yoshi-Egawa
Copy link
Contributor

Please answer these questions before submitting your issue. Thanks!

  1. What version of Python are you using?

    Python 3.11.8

  2. What operating system and processor architecture are you using?

    linux 64

  3. What are the component versions in the environment (pip freeze)?

    snowflake-snowpark-python==1.16.0

  4. What did you do?

    Let's say you have a custom class like the one at the bottom (a minimal example to check for errors).

from snowflake.snowpark.session import Session

connection_parameters: dict[str, int | str] = {
    "user": "<user_name>",
    "password": "<password>",
    "account": "<account_name>",
    "role": "<role_name>",
    "warehouse": "<warehouse_name>",
    "database": "<database_name>",
    "schema": "<schema_name>",
}

session = Session.builder.configs(connection_parameters).create()

df = session.create_dataframe(
    [["John", "Berry"], ["Rick", "Berry"], ["Anthony", "Davis"]],
    schema=["FIRST_NAME", "LAST_NAME"],
)

remote_file_path = f"{session.get_session_stage()}/names.parquet"

copy_result = df.write.copy_into_location(
    remote_file_path,
    file_format_type="parquet",
    header=True,
    overwrite=True,
    single=True,
)

When linting this file (Pylance v2024.4.1) errors appear.

No overloads for "copy_into_location" match the provided arguments

Argument of type "Literal[True]" cannot be assigned to parameter "overwrite" of type "str | None" in function "copy_into_location"
  Type "Literal[True]" cannot be assigned to type "str | None"
    "Literal[True]" is incompatible with "str"
    "Literal[True]" is incompatible with "None"

Argument of type "Literal[True]" cannot be assigned to parameter "single" of type "str | None" in function "copy_into_location"
  Type "Literal[True]" cannot be assigned to type "str | None"
    "Literal[True]" is incompatible with "str"
    "Literal[True]" is incompatible with "None"

image

  1. What did you expect to see?

    The type of copy_options in the copy_into_location method is Optional[str], but the type of copy_options in the CopyIntoLocationNode class used in the copy_into_location method is Dict[str, Any]. Consequently, to align the types, the copy_options in the copy_into_location method should be of type Optional[Any].

  2. Can you set logging to DEBUG and collect the logs?

    This is not a RunTime error, so I think this will not give us any extra information.

@Yoshi-Egawa Yoshi-Egawa added bug Something isn't working needs triage Initial RCA is required labels May 14, 2024
@github-actions github-actions bot changed the title Pylance throws errors when using copy_options with copy_into_location method SNOW-1417060: Pylance throws errors when using copy_options with copy_into_location method May 14, 2024
@Yoshi-Egawa
Copy link
Contributor Author

I made a mistake with this PR(#1372).

The copy_options of the copy_into_location method was supposed to be of type Optional[Any], but I changed it to Optional[Dict[str, Any]].

@sfc-gh-sghosh
Copy link

Hello @Yoshi-Egawa ,

The PR #1372 already merged, so you would like to undo the changes and propose a new fix ( The copy_options of the copy_into_location method was supposed to be of type Optional[Any], but I changed it to Optional[Dict[str, Any]].) ?

Regards,
Sujan

@sfc-gh-sghosh sfc-gh-sghosh added status-triage Issue is under initial triage and removed needs triage Initial RCA is required bug Something isn't working labels May 15, 2024
@Yoshi-Egawa
Copy link
Contributor Author

Hello @sfc-gh-sghosh ,

You're absolutely right. My apologies for the error in PR #1372. I've reverted the incorrect changes and opened a new pull request (PR #1575) with a more robust type definition for copy_options. This PR replaces the previous copy_options type with a stricter TypedDict definition, ensuring more accurate and robust type checking.

Thanks for catching that!
Yoshi

@sfc-gh-sghosh
Copy link

Hello @Yoshi-Egawa,

Thanks for submitting the PR (PR #1575).

Regards,
Sujan

@sfc-gh-sghosh sfc-gh-sghosh added status-triage_done Initial triage done, will be further handled by the driver team enhancement The issue is a request for improvement or a new feature and removed status-triage Issue is under initial triage labels May 16, 2024
@IraSil9
Copy link

IraSil9 commented Aug 18, 2024

@sfc-gh-sghosh @Yoshi-Egawa

Hello everyone.
I think there might still be an issue with this.

I'm using:
snowflake-snowpark-python = 1.20.0
(this is the latest version at the time of writing this comment)

The copy_into_location function expects **copy_options: Optional[Dict[str, Any]].
I provide: copy_options={"MAX_FILE_SIZE": 134217728}

On Snowflake side, the COPY statement itself gets translated like this:

COPY  INTO...
FROM ... 
FILE_FORMAT  = (  TYPE  = parquet )  
copy_options = {'MAX_FILE_SIZE': '134217728'}  
HEADER  = True

This produces an error since this is not the expected way to provide copy options.

In PR 1575 the type was supposed to be changed but this PR never got merged. It's closed.

@Yoshi-Egawa
Copy link
Contributor Author

Hello @sfc-gh-sghosh ,

Apologies, but it seems that I accidentally closed PR #1575. I’ve created a new PR (#1828) for the same changes. Could you kindly take a look at the new PR when you have a moment?

Thank you for your understanding and support!

Best regards,
Yoshi Egawa

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement The issue is a request for improvement or a new feature status-triage_done Initial triage done, will be further handled by the driver team
Projects
None yet
Development

No branches or pull requests

4 participants