Skip to content

Conversation

@arokem
Copy link
Member

@arokem arokem commented Dec 21, 2025

Replaces #113

Copilot AI review requested due to automatic review settings December 21, 2025 16:59
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR modernizes the project's linting infrastructure by replacing flake8 with ruff, a faster Python linter written in Rust. It also updates build system dependencies and adds pre-commit hooks for automated code quality checks.

Key changes:

  • Migration from flake8 to ruff for linting with custom rule configuration
  • Updated build system requirements (setuptools>=64, setuptools_scm>=8)
  • Addition of pre-commit configuration with ruff and codespell hooks

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 4 comments.

File Description
setup.cfg Updated build dependencies and added ruff to dev dependencies
pyproject.toml Added ruff configuration, updated build-system requirements, and modified pytest config section
.pre-commit-config.yaml Added new pre-commit configuration with ruff linting/formatting and codespell
.github/workflows/test.yml Replaced flake8 command with ruff check in CI workflow

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

sphinxcontrib-bibtex
myst-nb
wget
ruff>=0.14.10
Copy link

Copilot AI Dec 21, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The dev dependencies still include flake8 (line 65) even though it's being replaced by ruff. Since the linting command in the CI workflow is being changed to use ruff, flake8 should be removed from the dependencies to avoid confusion and reduce unnecessary dependencies.

Copilot uses AI. Check for mistakes.
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@copilot open a new pull request to apply changes based on this feedback

@arokem
Copy link
Member Author

arokem commented Dec 22, 2025

I believe this is ready for a review and merge, but no rush. It's a very large number of lines of code change, but it's almost exclusively changes in formatting or typos. The only substantive change is here:

def _is_int(val):
if isinstance(val, np.ndarray):
return False
if isinstance(val, int):
return True
elif isinstance(val, float):
if val.is_integer():
return True
else:
raise ValueError(f"n_seeds is set to {val}, which is not an integer")
def gen_seeds(
seed_mask,
seed_threshold,
n_seeds,
thresholds_as_percentages,
random_seeds,
rng_seed,
affine,
):
"""
Generate seeds for tracking. For parameters, see
:func:`AFQ.tractography.tractography.track`.
"""
logger.info("Generating Seeds...")
if _is_int(n_seeds):
# If it's float type, cast to integer:
if isinstance(n_seeds, float):
n_seeds = int(n_seeds)
, with relevant tests here:
def test_is_int():
npt.assert_(_is_int(5))
npt.assert_(_is_int(5.0))
npt.assert_(not _is_int(np.array([1, 2, 3])))
npt.assert_raises(ValueError, _is_int, 5.5)
. This change protects against cases where n_seeds input is provided as a float (with integer value), which otherwise raises an error later on. I saw this error show up on my laptop, even though doesn't show up on the CI, so this might be platform specific. BTW: I upgraded my latpop's OS to the most recent and now I am getting segfaults on our test suite 😅

@36000
Copy link
Collaborator

36000 commented Dec 27, 2025

This pull request looks great! I am super excited for this more clean organization for pyAFQ's codebase. Merging

@36000 36000 merged commit 12c29fa into tractometry:main Dec 27, 2025
9 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants