pre-commit / pre-commit-hooks

Some out-of-the-box hooks for pre-commit
MIT License
5.2k stars 694 forks source link

Feature: Add support for sections in requirements-fixer #1084

Closed MichaelAquilina closed 3 weeks ago

MichaelAquilina commented 3 weeks ago

Opening for discussion and tweaking. This is possibly also a feature we may want to put behind a flag in some way.

requirements-fixer has been very useful as a pre-commit hook but we have often found that it doesnt respect "sections" or requirements files that have been purposefully split from each other by new lines.

For example we might have something like

# global constraints should be set here
-c constraints.txt

# machine learning requirements go here
numpy
scikit-learn

# other requirements go here
requests
structlog

in this case we would want each of the sections to be sorted individually.

This PR adds this functionality to sort sections independently

asottile commented 3 weeks ago

please discuss features before wasting time on implementing them

I don't want to carry this complexity but thanks for taking the time to write it!

MichaelAquilina commented 3 weeks ago

please discuss features before wasting time on implementing them

in this case we implemented it before and are using the forked implementation so no waste :)

we thought it was worth starting that conversation with the implementation in use as a starting point

in terms of complexity - I didnt make too much of an effort to clean up the initial implementation in case there was some strong suggestions which need big changes anyway. But I do agree the code could be cleaned up overall

I think we're ok keeping the fork if this is not needed/wanted 👍🏼