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

[SPARK-30205][PYSPARK] Import ABCs from collections.abc to remove deprecation warnings #26835

Closed
wants to merge 1 commit into from
Closed

[SPARK-30205][PYSPARK] Import ABCs from collections.abc to remove deprecation warnings #26835

wants to merge 1 commit into from

Conversation

tirkarthi
Copy link
Contributor

@tirkarthi tirkarthi commented Dec 10, 2019

What changes were proposed in this pull request?

This PR aims to remove deprecation warnings by importing ABCs from collections.abc instead of collections.

Why are the changes needed?

This will remove deprecation warnings in Python 3.7 and 3.8.

$ python -V
Python 3.7.5

$ python python/pyspark/resultiterable.py
python/pyspark/resultiterable.py:23: DeprecationWarning:
Using or importing the ABCs from 'collections' instead of from 'collections.abc'
is deprecated since Python 3.3,and in 3.9 it will stop working
  class ResultIterable(collections.Iterable):

Does this PR introduce any user-facing change?

No, this doesn't introduce user-facing change

How was this patch tested?

Manually because this is about deprecation warning messages.

@tirkarthi tirkarthi changed the title [SPARK-30205] Import ABC from collections.abc for Python 3.9 compatibility [SPARK-30205][PYSPARK] Import ABC from collections.abc for Python 3.9 compatibility Dec 10, 2019
@dongjoon-hyun
Copy link
Member

ok to test

@SparkQA
Copy link

SparkQA commented Dec 10, 2019

Test build #115107 has finished for PR 26835 at commit b8a5304.

  • This patch passes all tests.
  • This patch merges cleanly.
  • This patch adds the following public classes (experimental):
  • class ResultIterable(Iterable):

Copy link
Member

@dongjoon-hyun dongjoon-hyun left a comment

Choose a reason for hiding this comment

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

Thank you for making your first PR, @tirkarthi .
However, Python 3.9 is not released yet. Can we revisit this as a single PR after Python 3.9 is officially released?

@tirkarthi
Copy link
Contributor Author

The change was meant to go as part of 3.8 but was postponed to 3.9 as noted in upstream PR due to Python ecosystem being incompatible. The PR also fixes the deprecation warning for all other versions like 3.8 and 3.7. I feel this is good enough to merge but will leave it to maintainers though.

@dongjoon-hyun
Copy link
Member

dongjoon-hyun commented Dec 10, 2019

For the following reason, it looks worth to me, too. Thank you for the explanation.

The PR also fixes the deprecation warning for all other versions like 3.8 and 3.7.

Could you change the PR title something like Remove deprecation warnings explicitly?
Never mind.

@dongjoon-hyun dongjoon-hyun changed the title [SPARK-30205][PYSPARK] Import ABC from collections.abc for Python 3.9 compatibility [SPARK-30205][PYSPARK] Import ABC from collections.abc to remove deprecation warnings Dec 10, 2019
@dongjoon-hyun
Copy link
Member

I will review this again with the Python versions supported by Apache Spark. Thanks!

@tirkarthi
Copy link
Contributor Author

Sure, thanks for the update.

@dongjoon-hyun
Copy link
Member

Hi, @tirkarthi . Could you check if this is the only required instance?

@dongjoon-hyun
Copy link
Member

I checked import collections and from collections a little. It looks okay.

Copy link
Member

@dongjoon-hyun dongjoon-hyun left a comment

Choose a reason for hiding this comment

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

+1, LGTM. Thank you for your first contribution, @tirkarthi .
Merged to master.

@dongjoon-hyun dongjoon-hyun changed the title [SPARK-30205][PYSPARK] Import ABC from collections.abc to remove deprecation warnings [SPARK-30205][PYSPARK] Import ABCs from collections.abc to remove deprecation warnings Dec 10, 2019
@dongjoon-hyun
Copy link
Member

Thank you, @tirkarthi . (Karthikeyan Singaravelan). You are added to the Apache Spark contributor group and SPARK-30205 is assigned to you.

@tirkarthi
Copy link
Contributor Author

Thank you :)

try:
from collections.abc import Iterable
except ImportError:
from collections import Iterable
Copy link
Member

Choose a reason for hiding this comment

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

I know this way is more preferred but can we do this by version-checking? That's consistent in the code base at least.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sorry, I was not aware of it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
4 participants