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.

Sign up for GitHub

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

Jump to bottom

add ignore for the recently added and failing mypy error 'type-var' SPARK-39811 #37210

Closed
wants to merge 1 commit into from
Closed

add ignore for the recently added and failing mypy error 'type-var' SPARK-39811 #37210

wants to merge 1 commit into from

Conversation

anilbey
Copy link

@anilbey anilbey commented Jul 17, 2022 •
edited
Loading

A new error introduced to mypy is found in this repository. This PR adds comment to ignore that error.

spark (https://github.com/apache/spark)

  • python/pyspark/ml/wrapper.py:267: error: A function returning TypeVar should receive at least one argument containing the same Typevar [type-var]

Reference:
python/mypy#13166

Jira Issue:
https://issues.apache.org/jira/browse/SPARK-39811

@AmplabJenkins
Copy link

Can one of the admins verify this patch?

HyukjinKwon
HyukjinKwon reviewed Jul 17, 2022
View reviewed changes
python/pyspark/ml/wrapper.py
@@ -264,7 +264,7 @@ def _to_java(self) -> "JavaObject":
return self._java_obj

@staticmethod
def _from_java(java_stage: "JavaObject") -> "JP":
def _from_java(java_stage: "JavaObject") -> "JP": # type: ignore[type-var]
Copy link
Member

Choose a reason for hiding this comment

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

@anilbey mind filing a JIRA and linking it into PR title? See also, https://spark.apache.org/contributing.html

Copy link
Author

Choose a reason for hiding this comment

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

@HyukjinKwon here it is
https://issues.apache.org/jira/browse/SPARK-39811

Copy link
Member

@HyukjinKwon HyukjinKwon Jul 19, 2022 •
edited
Loading

Choose a reason for hiding this comment

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

Let's link it to the PR title, and also please take a look at https://github.com/apache/spark/pull/37210/checks?check_run_id=7379165026

Copy link
Author

Choose a reason for hiding this comment

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

The title now contains "SPARK-39811". Rebasing didn't make a difference on the CI failure.

@anilbey
Copy link
Author

anilbey commented Jul 18, 2022

Please note that ignoring the type error is not the best solution.

Ideally the return type should be changed to avoid this error in the first place.

@anilbey anilbey changed the title add ignore for the recently added and failing mypy error 'type-var' add ignore for the recently added and failing mypy error 'type-var' SPARK-39811 Jul 19, 2022
@itholic
Copy link
Contributor

itholic commented Sep 1, 2022 •
edited
Loading

Hi, @anilbey ! Thanks for the contribution for PySpark.

Seems like you might need to address some stuffs as below:

  • For running the test properly, could you check the "Workflow run detection failed" in the https://github.com/apache/spark/pull/37210/checks?check_run_id=7404721464 ? You should enable the Github Actions workflow running on your forked repository to run the CI.

  • Could you fix the title in the proper format? It should be of the form [SPARK-xxxx][COMPONENT] Title, which is described in https://spark.apache.org/contributing.html.

    • e.g. [SPARK-39811][PYTHON] add ignore for the recently added and failing mypy error 'type-var'.

  • Let's use the existing PR description format without any modifying. It should be of the form:

### What changes were proposed in this pull request?

### Why are the changes needed?

### Does this PR introduce _any_ user-facing change?

### How was this patch tested?

@github-actions
Copy link

We're closing this PR because it hasn't been updated in a while. This isn't a judgement on the merit of the PR in any way. It's just a way of keeping the PR queue manageable.
If you'd like to revive this PR, please reopen it and ask a committer to remove the Stale tag!

@github-actions github-actions bot added the Stale label Dec 11, 2022
@github-actions github-actions bot closed this Dec 12, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@HyukjinKwon HyukjinKwon HyukjinKwon left review comments

Assignees
No one assigned
Labels
CORE ML PYTHON Stale
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@anilbey @AmplabJenkins @itholic @HyukjinKwon