networkx / nx-parallel

A networkx backend that uses joblib to run graph algorithms in parallel.
BSD 3-Clause "New" or "Revised" License
23 stars 20 forks source link

MAINT: adding `lint.select` and corresponding style fixes #67

Closed Schefflera-Arboricola closed 1 month ago

Schefflera-Arboricola commented 4 months ago

Will open this PR after PR#69 is merged

This PR switches the nx-parallel's building tool from hatchling to setuptools as it is more widely used and it's better for bigger projects and we need to explicitly mention all the packages(unlike hatchling) so that ensures that all packages are being built.

I didn't add any extra ruff or lint settings. But, the style wf test was failing in github wf so in the last commit in this PR are all the pre-commit changes. I think it is a result of switching from hatchling to setuptools.

moved select, pre-file-ignores under tool.ruff.lint section due to the following warning:

warning: The top-level linter settings are deprecated in favour of their counterparts in the `lint` section. Please update the following options in `pyproject.toml`:
  - 'select' -> 'lint.select'
  - 'per-file-ignores' -> 'lint.per-file-ignores'

dschult commented 4 months ago

The changes to pyproject.toml look like this better matches networkx. The import changes and package names all look good. Perhaps @jarrodmillman or @MridulS can look at the setuptools settings to make sure they make sense.

Can you add a few sentences to the PR's first post describing what you did? (use the "Edit" option under the 3-dots at the upper right of that first post.) That way reviewers don't have to click somewhere else to figure out what this PR is about.

Just some very basic info (not all has to be there and more obviously can be there): for example:

What?

Why?

How?

Testing?

(example from https://www.pullrequest.com/blog/writing-a-great-pull-request-description/)

For most PRs I look for "What" and "Why" in the description. Jarrod has the release process set up so the title gets added to the release notes (I believe), so that's important too. (As a reviewer I try to remember to update the title before merging to make it helpful for the release notes.)

Schefflera-Arboricola commented 3 months ago

The pyproject.toml file has all the "real" changes and all other files have linting changes. And all the linting modifications are in the order of the import statements. I think this was a result of switching to setuptools because the only new linting specification I added was:

[tool.ruff.format]
docstring-code-format = true

and this is for the styling of docstrings.

it would also allow us to keep the blame clean by ignoring the formatting-only changes!

I think this seems reasonable but idk what would be a quick way to revert these automated changes in this PR. So, I'll make a new PR that changes the pyproject.toml, and then after merging that PR, I'll rebase this PR's branch and then we can merge this. I hope that would be ok. LMK what you think.