Skip to content

Conversation

@sethkimmel3
Copy link
Contributor

@sethkimmel3 sethkimmel3 commented Mar 1, 2023

Summary & Motivation

Fixes #12604

.iteritems is deprecated since 1.50 and will be removed in a future version. Changing to .items. (pandas-dev/pandas#45321).

How I Tested These Changes

I did not, but pandas-dev/pandas#45321 appears to indicate it's a safe change to make. If I can somehow install this version of dagster on my machine, I'm happy to test using the code that surfaced the issue, but I'm not sure how to do that.

@vercel
Copy link

vercel bot commented Mar 1, 2023

The latest updates on your projects. Learn more about Vercel for Git ↗︎

1 Ignored Deployment
Name Status Preview Comments Updated
dagit-storybook ⬜️ Ignored (Inspect) Mar 1, 2023 at 10:06PM (UTC)

@vercel
Copy link

vercel bot commented Mar 1, 2023

@sethkimmel3 is attempting to deploy a commit to the Elementl Team on Vercel.

A member of the Team first needs to authorize it.

@sryza
Copy link
Contributor

sryza commented Mar 1, 2023

@sethkimmel3 do you know when items was added? Just want to make sure we're not breaking people who are on older-but-still-used Pandas versions

@sethkimmel3
Copy link
Contributor Author

sethkimmel3 commented Mar 1, 2023

@sryza looks like it's been around since 0.5.0 (or earlier): https://github.com/pandas-dev/pandas/blob/v0.5.0/pandas/core/series.py#L447 (pending python3 compatibility). Would probably be a good time to upgrade if they're still on a version that old!

@sryza
Copy link
Contributor

sryza commented Mar 1, 2023

Great - that's a long time. This LGTM. Kicking off a build.

@sryza
Copy link
Contributor

sryza commented Mar 1, 2023

pyright has this complaint:

[2023-03-01T20:56:02Z] /workdir/python_modules/libraries/dagster-snowflake-pandas/dagster_snowflake_pandas/snowflake_pandas_type_handler.py:
[2023-03-01T20:56:02Z]   79:41: Argument of type "Hashable" cannot be assigned to parameter "name" of type "str" in function "__new__"
[2023-03-01T20:56:02Z]   "Hashable" is incompatible with "str" (reportGeneralTypeIssues)

To avoid this, I think probably just casting to a string is best?

@sethkimmel3
Copy link
Contributor Author

sethkimmel3 commented Mar 1, 2023

@sryza that seems sensible. But I'm not sure why it would flag this as a new issue, as it appears .iteritems has just been an alias for .items and calls that function directly. What version of pandas is dagster currently using?

@sryza
Copy link
Contributor

sryza commented Mar 1, 2023

What version of pandas is dagster currently using?

I don't believe we pin any particular version. In our CI, I believe we use the latest version (just pip install pandas).

@sethkimmel3
Copy link
Contributor Author

I'm assuming this is what you meant by casting to str. Let's see if the build likes this!

@sryza sryza self-requested a review March 1, 2023 23:20
Copy link
Contributor

@sryza sryza left a comment

Choose a reason for hiding this comment

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

Looks good!

@sryza sryza merged commit 3fae108 into dagster-io:master Mar 1, 2023
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.

FutureWarning in dagster_snowflake_pandas

2 participants