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

FIX-#3371: Remove pandas patch level pin #6211

Conversation

mvashishtha
Copy link
Collaborator

@mvashishtha mvashishtha commented May 25, 2023

What do these changes do?

Remove pandas patch level pin as we agreed here.

  • first commit message and PR title follow format outlined here

    NOTE: If you edit the PR title to match this format, you need to add another commit (even if it's empty) or amend your last commit for the CI job that checks the PR title to pick up the new PR title.

  • passes flake8 modin/ asv_bench/benchmarks scripts/doc_checker.py
  • passes black --check modin/ asv_bench/benchmarks scripts/doc_checker.py
  • signed commit with git commit -s
  • Resolves Do not pin pandas down to patch level #3371
  • tests added and passing
  • module layout described at docs/development/architecture.rst is up-to-date

@mvashishtha mvashishtha force-pushed the 3371/fix/remove-pandas-patch-level-pin branch from 64bf46d to 35374b6 Compare June 8, 2023 12:43
@mvashishtha mvashishtha marked this pull request as ready for review June 8, 2023 14:03
@mvashishtha mvashishtha requested a review from a team as a code owner June 8, 2023 14:03
environment-dev.yml Outdated Show resolved Hide resolved
setup.py Outdated
@@ -47,7 +47,7 @@ def make_distribution(self):
long_description=long_description,
long_description_content_type="text/markdown",
install_requires=[
"pandas==2.0.2",
"pandas>1,<2.1",
Copy link
Collaborator

Choose a reason for hiding this comment

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

Imagine a case when the user has pandas=1.3.5 and python 3.7. Then he wants to install modin. However, Modin only supports python>=3.8. Maybe we should remove pandas patch level pin starting 2.0?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

users on pandas 1.3.5 and python 3.7 can't use the latest version of modin anyway because modin now requires python >= 3.8. I think this is a separate issue. What do you suggest that we change?

I suspect that even importing the latest modin code on python 3.7 would break because we make so many assumptions about what we can import from pandas. That was why backwards compatibility with python 3.6 was so much work. We should only consider backwards compatibility with python 3.7 if we know of a desire for it.

Copy link
Collaborator

Choose a reason for hiding this comment

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

The changes now are fine by me.

mvashishtha and others added 3 commits June 9, 2023 07:51
Co-authored-by: Anatoly Myachev <[email protected]>
…vashishtha/modin into 3371/fix/remove-pandas-patch-level-pin
@mvashishtha
Copy link
Collaborator Author

@YarShev @anmyachev ready for merge

@YarShev YarShev merged commit af11ce6 into modin-project:master Jun 9, 2023
@vnlitvinov
Copy link
Collaborator

Do note that this change is potentially opening a can of worms for us because we use some internal pandas things (be them functions, classes or variables), for which pandas is free to change or even remove those without breaking SemVer logic (as they're not public API).

But then, we can always pin version again if such thing happens...

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.

Do not pin pandas down to patch level
4 participants