Skip to content

CI: Remove redundant check #57156

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

Merged
merged 1 commit into from
Jan 31, 2024
Merged

Conversation

tqa236
Copy link
Contributor

@tqa236 tqa236 commented Jan 30, 2024

I removed 2 checks that's already covered in ruff's PGH003 and PGH004

The second commit is just an example that the check works. Will revert before merging if approved.

ruff....................................................................................................Failed
- hook id: ruff
- exit code: 1

pandas/compat/numpy/function.py:209:18: PGH003 Use specific rule codes when ignoring type issues
pandas/tests/computation/test_compat.py:30:18: PGH004 Use specific rule codes when using `noqa`
Found 2 errors.
  • closes #xxxx (Replace xxxx with the GitHub issue number)
  • Tests added and passed if fixing a bug or adding a new feature
  • All code checks passed.
  • Added type annotations to new arguments/methods/functions.
  • Added an entry in the latest doc/source/whatsnew/vX.X.X.rst file if fixing a bug or adding a new feature.

@datapythonista datapythonista added the CI Continuous Integration label Jan 31, 2024
@datapythonista datapythonista changed the title Remove redundant check CI: Remove redundant check Jan 31, 2024
@datapythonista
Copy link
Member

Thanks for the work on this @tqa236. I only see one of the errors failing in the CI. Is this working as expected?

@tqa236
Copy link
Contributor Author

tqa236 commented Jan 31, 2024

hello @datapythonista , the CI works as expected. The 2 errors are caught by ruff and made the pre-commit job failed, as shown here. In addition, I think recently even mypy will also catch the bare type ignore error, and raise that error again in the mypy check.

mypy.....................................................................Failed
- hook id: mypy
- duration: 77.99s
- exit code: 1

pandas/compat/numpy/function.py:209: error: "type: ignore" comment without error code (consider "type: ignore[return-value]" instead)  [ignore-without-code]
Found 1 error in 1 file (checked 1449 source files)

Copy link
Member

@datapythonista datapythonista 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 clarifying @tqa236, I was checking the Code checks job in GitHub actions, I didn't see that. Good to see this simplified.

Looks good to me then, feel free to revert the errors.

@tqa236 tqa236 force-pushed the remove-redundant-check branch from 083efd5 to 4754b86 Compare January 31, 2024 11:00
@tqa236
Copy link
Contributor Author

tqa236 commented Jan 31, 2024

thanks for the review, @datapythonista, I reverted the last commit. The PR is ready.

@mroeschke mroeschke added this to the 3.0 milestone Jan 31, 2024
@mroeschke mroeschke added the Code Style Code style, linting, code_checks label Jan 31, 2024
@mroeschke mroeschke merged commit a3e4391 into pandas-dev:main Jan 31, 2024
@mroeschke
Copy link
Member

Thanks again @tqa236

If interested, it would be great to see what ruff (pylint) checks are covered by what is run by pylint. It would be great if we could just switch over to ruff with most of the rules being covered

@tqa236
Copy link
Contributor Author

tqa236 commented Jan 31, 2024

@mroeschke I can look into it. Do you mind if I bump ruff to the latest version? A lot of new pylint rules are added recently, but I was trying to avoid creating conflict with your work in #56863

@tqa236 tqa236 deleted the remove-redundant-check branch January 31, 2024 19:01
@mroeschke
Copy link
Member

Do you mind if I bump ruff to the latest version?

Go for it. I am happy to keep that branch updated with whatever changes you make

pmhatre1 pushed a commit to pmhatre1/pandas-pmhatre1 that referenced this pull request May 7, 2024
Remove redundant code style check
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CI Continuous Integration Code Style Code style, linting, code_checks
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants