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 mypy check error: pandas merge copy param #105

Merged

Conversation

shenxiangzhuang
Copy link
Contributor

@shenxiangzhuang shenxiangzhuang commented May 27, 2024

Fix #104

Checklist

  • I have read the CONTRIBUTING document
  • I hereby agree to the terms of either individual CLA available at: https://toloka.ai/legal/cla_individual or corporate CLA available at: https://toloka.ai/legal/cla_corporate, whichever is applicable to me, as it pertains to direct contributions to crowd-kit only
  • My change requires a change to the documentation
  • I have updated the documentation accordingly
  • I have added tests to cover my changes
  • All new and existing tests passed

@shenxiangzhuang shenxiangzhuang changed the title remove the param Fix pandas merge copy param check error May 27, 2024
@shenxiangzhuang shenxiangzhuang changed the title Fix pandas merge copy param check error Fix mypy check error: pandas merge copy param May 27, 2024
@dustalov
Copy link
Collaborator

Hi, the last time I tried, every type check passed well. Why turning on copying fixes it? It might increase the memory usage as it is performed at each E-step.

@shenxiangzhuang
Copy link
Contributor Author

shenxiangzhuang commented May 30, 2024

Hi, the last time I tried, every type check passed well. Why turning on copying fixes it? It might increase the memory usage as it is performed at each E-step.

The error is from mypy checking process as showed in #104, which come from the latest update of pandas-stubs. And I agree that this change will increase the memory usage, maybe we can just add # type: ignore after this line.

Maybe we can just close this pr and related issue for now and talk about it until we reproduce the ci error in the repo.

@pilot7747
Copy link
Contributor

I'd either add # type: ignore or figure out how to make it pass mypy checks without copying.

@dustalov
Copy link
Collaborator

These stubs must have been ahead of time. Let’s add the corresponding type: ignore[call-arg] comment then.

@shenxiangzhuang
Copy link
Contributor Author

These stubs must have been ahead of time. Let’s add the corresponding type: ignore[call-arg] comment then.

Using # type: ignore[call-arg, unused-ignore] instead to avoid introduce another check error like fellows:

crowdkit/aggregation/classification/glad.py:161: error: Unused "type: ignore" comment  [unused-ignore]
Found 1 error in 1 file (checked 79 source files)
Error: Process completed with exit code 1.

@shenxiangzhuang
Copy link
Contributor Author

I'd either add # type: ignore or figure out how to make it pass mypy checks without copying.

Hi @pilot7747 , thanks for your reply! I just give a suggestion here and feel free to close this pr without merge if you'd like another solution.

crowdkit/aggregation/classification/glad.py Outdated Show resolved Hide resolved
@dustalov
Copy link
Collaborator

Wait, if there is unused-ignore error, then the call-arg is not the problem here.

@shenxiangzhuang
Copy link
Contributor Author

Wait, if there is unused-ignore error, then the call-arg is not the problem here.

https://github.com/Toloka/crowd-kit/pull/105/checks?check_run_id=25642305515
The unused-ignore error only raised in Python3.8, while works well in Python version >=3.9. So I think it's ok here to use # type: ignore[call-arg, unused-ignore] here.

@dustalov
Copy link
Collaborator

Ah, I see, thanks! Please remove the space after the comma in the type comment, and we'll merge it immediately.

@dustalov dustalov merged commit faa071f into Toloka:main May 31, 2024
5 checks passed
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.

[BUG] Mypy check error in newest pandas-stubs version
3 participants