Skip to content

CI matrix, linting, CLI parsing, and modernize type hints + flowfield/numba fixes#8

Closed
sergioald wants to merge 1 commit into
mainfrom
codex/identify-improvements-for-repository
Closed

CI matrix, linting, CLI parsing, and modernize type hints + flowfield/numba fixes#8
sergioald wants to merge 1 commit into
mainfrom
codex/identify-improvements-for-repository

Conversation

@sergioald

@sergioald sergioald commented Jun 19, 2026

Copy link
Copy Markdown
Owner

Motivation

  • Modernize the codebase for newer Python versions, improve developer experience with linting/formatting, and make CLI case selection more robust and user-friendly.
  • Improve performance-safety and clarity in the flow-field / numba codepaths and ensure deterministic parallel behaviour and clearer warnings.
  • Add a console entry point and tighten a few numeric/validation failure modes to avoid surprising runtime errors.

Description

  • CI and tooling: expand the GitHub Actions smoke-tests job to a Python-version matrix (3.10, 3.11, 3.12) and parameterize actions/setup-python; add ruff as a pre-commit hook with --fix enabled and register a ldsfl-run script entry in pyproject.toml.
  • CLI and runner: replace run_ldsfl.parse_cases with a stricter parser that supports ranges (e.g. 1,3-5), de-duplicates and validates input, expose it as an argparse type, and update run_ldsfl.main to use it; add more robust parsing tests and update smoke/integration tests accordingly.
  • Type hints and API modernisation: migrate many typing annotations to PEP 604/explicit tuple[...] and X | None styles, remove unused imports, and modernise return annotations (e.g. -> tuple[...]) across flowfield, periodic flowfield, semiana, geometry, and other modules.
  • Flow-field / numba and numerical fixes: rearrange imports, add stricter checks and clearer RuntimeWarning usage (with stacklevel), avoid large temporary allocations, use zip(..., strict=True) where appropriate, and tidy ThreadPoolExecutor worker typing and behaviour; add safer checks for non-finite migration speed in dxdy2.
  • Miscellaneous: reorder and tidy imports in GUI modules, fix small bugs/unused variables in resistance.py, move some numpy/pandas imports to top-level modules, and add a more informative final-warning stacklevel when saving snapshots fails.

Testing

  • Ran pre-commit run --all-files locally to apply formatting/lint fixes (hook added to repo); this completed successfully.
  • Executed the test-suite with pytest -q and coverage (python -m pytest --cov=ldsfl --cov-report=term-missing --cov-report=xml), and the updated unit tests (including tests/test_cli_config.py) passed.
  • CI will run the smoke-tests matrix across Python 3.10/3.11/3.12 (configured in .github/workflows/tests.yml) which runs pre-commit and pytest as described above.

@sergioald
sergioald marked this pull request as ready for review June 19, 2026 22:16

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

💡 Codex Review

name: coverage-xml

P1 Badge Use a matrix-specific artifact name

In this workflow, all three python-version matrix jobs run the actions/upload-artifact@v4 step and try to create the same coverage-xml artifact. The upload-artifact v4 documentation says artifact names must be unique and matrix jobs with the same name will hit conflict errors (https://github.com/actions/upload-artifact#not-uploading-to-the-same-artifact), so two of the three CI legs will fail after tests pass unless the name includes ${{ matrix.python-version }} or similar.

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread pyproject.toml
]

[project.scripts]
ldsfl-run = "run_ldsfl:main"

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 Badge Ship the module targeted by the console script

The new ldsfl-run entry point imports run_ldsfl:main, but this project’s setuptools config only discovers packages matching ldsfl* under [tool.setuptools.packages.find], so the top-level run_ldsfl.py module is not included in a normal wheel. Setuptools only performs automatic py_modules discovery when both packages and py_modules are left unspecified (https://setuptools.pypa.io/en/latest/userguide/package_discovery.html#automatic-discovery), so installed users will get ModuleNotFoundError: run_ldsfl when invoking ldsfl-run unless the module is moved into ldsfl or explicitly listed as a py-module.

Useful? React with 👍 / 👎.

@sergioald sergioald closed this Jun 20, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant