Add Python linter to github actions

[lshamis]

Adding black to the CI as requested here: #949
This PR only adds the check, it does not format. That would make this change larger than I'd like.

Note, that it finds unusual code in tools/star_rod_idx_to_c.py:422

error: cannot format tools/star_rod_idx_to_c.py: cannot use --safe with this file; failed to parse source file AST: f-string: invalid syntax (<unknown>, line 422)

[ethteck]

Thanks for the PR! A few pieces of feedback

Also, does the new check pass currently? If not, this pr should probably introduce the changes required for it to do so

[ethteck]

Also, black is a formatter rather than a linter. For linting, we use mypy. Though I imagine getting that set up and fixing all issues is a lot more work, so we can tackle that later. I'd just rename the pr / step in the yaml to say format rather than lint

[lshamis]

Also, black is a formatter rather than a linter. For linting, we use mypy. Though I imagine getting that set up and fixing all issues is a lot more work, so we can tackle that later. I'd just rename the pr / step in the yaml to say format rather than lint

Renamed py_lint -> py_format
Changed name Python format and lint -> Python format

[lshamis]

Also, does the new check pass currently? If not, this pr should probably introduce the changes required for it to do so

If you like, I can include the ~60 files that would be reformatted.

It would still not pass, though, due to tools/star_rod_idx_to_c.py:422
That line doesn't look like valid Python to me.

[ethteck]

Could you add the files, and then I'll fix the issue with that one script?

[lshamis]

Could you add the files, and then I'll fix the issue with that one script?

Done

[ethteck]

Since we have the "Lint" workflow off (since c linting is a bag of worms we haven't really figured out yet for this project), this won't run after we merge it. I think it'd be fine if we made a Python Lint workflow (a new yaml) and put this there, which we could enable. Would you like me to do that, or do you want to?

[ethteck]

I'm also observing that only the most recent instance --exclude takes effect. So if I exclude diff.py last, it'll exclude that, and if I exclude another thing last, diff.py says it'll be reformatted. I think we want --extend-exclude for subsequent uses of that flag

[marijnvdwerf]

Wouldn't excludes usually be defined in pyproject.toml? That way you can also run black locally and have it ignore the same files as CI.

[ethteck]

I'm not sure, but this isn't really a python project so it seems weird to have such a file for it. I also wouldn't know how to set it up to work with black, but I guess if someone wants to do that, that's cool

[lshamis]

I'm not going to be able to make any changes for another day or so. Feel free to make any further changes and commit without me.

[ethteck]

shounds good. Thanks for this effort! Definitely long overdue

[bates64]

Wouldn't excludes usually be defined in pyproject.toml? That way you can also run black locally and have it ignore the same files as CI.

This would be worth doing before merge I think, to support other editors

[bates64]

good pr though, thanks!