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

STYLE use yaml anchors in .pre-commit-config.yaml for flake8 #43282

Closed
MarcoGorelli opened this issue Aug 29, 2021 · 15 comments · Fixed by #44177
Closed

STYLE use yaml anchors in .pre-commit-config.yaml for flake8 #43282

MarcoGorelli opened this issue Aug 29, 2021 · 15 comments · Fixed by #44177
Labels
Code Style Code style, linting, code_checks good first issue

Comments

@MarcoGorelli
Copy link
Member

Instead of duplicating these lines

additional_dependencies:
- flake8-comprehensions==3.1.0
- flake8-bugbear==21.3.2
- pandas-dev-flaker==0.2.0

additional_dependencies:
- flake8==3.9.2
- flake8-comprehensions==3.1.0
- flake8-bugbear==21.3.2
- pandas-dev-flaker==0.2.0

we would use a yaml anchor

See here for an example of how that works https://github.com/psf/black/pull/2459/files


This would also mean being able to get rid of a big part of https://github.com/pandas-dev/pandas/blob/e39ea3024cebb4e7a7fd35972a44637de6c41650/scripts/sync_flake8_versions.py

@MarcoGorelli MarcoGorelli added Code Style Code style, linting, code_checks good first issue labels Aug 29, 2021
@lorenzophys
Copy link
Contributor

@MarcoGorelli a question from a non yaml expert: how would you merge those two lists? I don't believe this will work because additional_dependencies will become nested.

additional_dependencies:  &version_check_dependencies [
    flake8-comprehensions==3.1.0,
    flake8-bugbear==21.3.2,
    pandas-dev-flaker==0.2.0,
]
...
additional_dependencies: 
    - flake8==3.9.2
    - *version_check_dependencies

I cannot think of a way to get the second additional_dependencies to be a list.

@Kunsh-Manghwani
Copy link

I have resolved the issue and generated a PR .

@Anupam-USP
Copy link
Contributor

Is this issue still open?

@MarcoGorelli
Copy link
Member Author

yup!

@Anupam-USP
Copy link
Contributor

Can i take? @Kunsh-Manghwani has generated a PR

@MarcoGorelli
Copy link
Member Author

Yes, you can take it, their PR hasn't been active for a while

@MarcoGorelli
Copy link
Member Author

Here's another example: asottile/yesqa#49 (comment)

@NavjotBansal
Copy link

I see this issue is pending since long
I am working on this in parallel if nobody raises a PR for 2-3 more days I would do that 😄

@MarcoGorelli
Copy link
Member Author

Awesome, feel free to open a PR when you want

@NavjotBansal
Copy link

@MarcoGorelli Sure

@trallard
Copy link
Contributor

Hey folks I had a quick look at this and found actually that this does not seem immediately fixable. From the .precommit-config.yaml

 additional_dependencies: 
     - flake8-comprehensions==3.1.0 
     - flake8-bugbear==21.3.2 
     - pandas-dev-flaker==0.2.0 
 additional_dependencies: 
   - flake8==3.9.2 
   - flake8-comprehensions==3.1.0 
   - flake8-bugbear==21.3.2 
   - pandas-dev-flaker==0.2.0 

This entails the creation of an anchor and then merging two lists, however, this is not yet possible. for reference

You can see
yaml/pyyaml#469
yaml/yaml#35
yaml/yaml#48

The examples listed elsewhere in this issue can use an anchor since they reuse the same dependencies without the need of merging/extending lists

@MarcoGorelli
Copy link
Member Author

I see, thanks for looking into this!

I'll have another look next week, but this might need closing by the looks of it

@Skylion007
Copy link

Skylion007 commented Oct 14, 2021

You can just use:

 additional_dependencies: 
   - flake8==3.9.2 
   - flake8-comprehensions==3.1.0 
   - flake8-bugbear==21.3.2 
   - pandas-dev-flaker==0.2.0 

for both additional_dependencies. It's not like the flake8 additional requirement will conflict with the flake8 hook. (Just make sure to keep it updated).

@trallard
Copy link
Contributor

I suppose that is the easiest way around - if y'all are happy with that I can send a quick PR

@MarcoGorelli
Copy link
Member Author

Seems good

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Code Style Code style, linting, code_checks good first issue
Projects
None yet
Development

Successfully merging a pull request may close this issue.

7 participants