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

Ignore E203 in flake8 #149

Open
surchs opened this issue May 1, 2024 · 6 comments
Open

Ignore E203 in flake8 #149

surchs opened this issue May 1, 2024 · 6 comments
Labels
maint:coverage Test coverage improvements that were not included in feature prioritization quick fix Minimal planning and/or implementation work required. someday Not a priority right now, but we want to keep this around to think or discuss more. type:maintenance Upkeeping efforts & catch-up corrective improvements that are not Features nor Bugs

Comments

@surchs
Copy link
Contributor

surchs commented May 1, 2024

flake8 causes false positives of E203 (https://www.flake8rules.com/rules/E203.html) in certain cases

Since black handles this correctly (afaict), let's ignore rule 203 and get the linters to calm down a bit

@surchs surchs added type:maintenance Upkeeping efforts & catch-up corrective improvements that are not Features nor Bugs maint:coverage Test coverage improvements that were not included in feature prioritization flag:schedule Flag issue that should go on the roadmap or backlog. labels May 1, 2024
@surchs
Copy link
Contributor Author

surchs commented May 1, 2024

This will also let us merge neurobagel/bagel-cli#297

@surchs
Copy link
Contributor Author

surchs commented May 1, 2024

One weird aspect is that I haven't been able to reproduce these false positives from flake8 locally. I.e. they only seem to happen as part of the pre-commit CI run

@surchs surchs added quick fix Minimal planning and/or implementation work required. and removed flag:schedule Flag issue that should go on the roadmap or backlog. labels May 1, 2024
@alyssadai
Copy link

Can we close this since neurobagel/bagel-cli#300 is merged?

@alyssadai alyssadai transferred this issue from neurobagel/documentation May 6, 2024
@alyssadai
Copy link

Ignore rule needs to be added to all repos where pre-commit-CI is set up

@surchs
Copy link
Contributor Author

surchs commented Aug 13, 2024

is there going to be enough overlap on the ignore rules to sync them to all (python) repos? I would assume so

@surchs
Copy link
Contributor Author

surchs commented Sep 19, 2024

There may not be enough overlap to just sync .precomit-config across all python repos. Let's revisit this issue when the E203 error pops up again. Likely at that point we would address it more generally by making the python repo setup consistent across our repos. That would be more of a maintenance milestone probably

@surchs surchs added the someday Not a priority right now, but we want to keep this around to think or discuss more. label Sep 19, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
maint:coverage Test coverage improvements that were not included in feature prioritization quick fix Minimal planning and/or implementation work required. someday Not a priority right now, but we want to keep this around to think or discuss more. type:maintenance Upkeeping efforts & catch-up corrective improvements that are not Features nor Bugs
Projects
Status: No status
Development

No branches or pull requests

2 participants