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

Exclude pyrsistent version 0.19.1 from deps #356

Merged
merged 7 commits into from Nov 14, 2022

Conversation

namanjain
Copy link
Collaborator

Pyrsistent 0.19.1 had a regression in 0.19.1 which was fixed in 0.19.2
The regression was on pmap where inserted elements were unreliably
inserted. See tobgu/pyrsistent#263

Pyrsistent 0.19.1 had a regression in 0.19.1 which was fixed in 0.19.2
The regression was on pmap where inserted elements were unreliably
inserted. See tobgu/pyrsistent#263
@namanjain namanjain force-pushed the naman/exclude-pyrsistent-version branch 9 times, most recently from 4ae8dd1 to 735d2f1 Compare November 4, 2022 18:07
@namanjain namanjain force-pushed the naman/exclude-pyrsistent-version branch from 735d2f1 to 8ae96ad Compare November 4, 2022 20:33
Copy link
Collaborator

@jqmp jqmp left a comment

Choose a reason for hiding this comment

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

Thanks for cleaning up all the 3.6 stuff! (BTW, I assume no one at Square still depends on 3.6? I think we originally supported it because we needed it for AI Platform or something.)

@@ -22,7 +22,8 @@
"numpy",
"pandas",
"pyarrow",
"pyrsistent",
"pyrsistent!=0.19.1",
"decorator<5",
Copy link
Collaborator

Choose a reason for hiding this comment

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

Could we add a comment explaining the conflict here?

Copy link

@andrewblane andrewblane left a comment

Choose a reason for hiding this comment

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

LGTM, though I think the package version needs to be bumped.

@namanjain
Copy link
Collaborator Author

Thanks for cleaning up all the 3.6 stuff! (BTW, I assume no one at Square still depends on 3.6? I think we originally supported it because we needed it for AI Platform or something.)

Yes, I asked in internal slack and 3.6 is not used anymore. Existing pipelines that still are on 3.6 will continue to use the older versions of bionic.

@namanjain
Copy link
Collaborator Author

LGTM, though I think the package version needs to be bumped.

Yes, I'll open a new PR for bumping version following the release process https://bionic.readthedocs.io/en/stable/maintaining.html?highlight=release#release-process

@namanjain namanjain merged commit c92e4d0 into master Nov 14, 2022
@namanjain namanjain deleted the naman/exclude-pyrsistent-version branch November 14, 2022 23:51
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

3 participants