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

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

Conversation

MichaelAquilina
Copy link

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

@MichaelAquilina MichaelAquilina force-pushed the feat/requirements-sections branch from 27f60ee to cc5d450 Compare August 21, 2024 15:16
@asottile
Copy link
Member

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!

@asottile asottile closed this Aug 21, 2024
@MichaelAquilina
Copy link
Author

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 👍🏼

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

2 participants