Skip to content

Fix magnitude scaling module to allow array types for magnitude to area functions #56

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.

Sign up for GitHub

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

Open
lispandfound wants to merge 14 commits into
base: main
Choose a base branch
from
Open

Fix magnitude scaling module to allow array types for magnitude to area functions #56

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
14 commits
Select commit Hold shift + click to select a range
a79273e
Reapply "fix(magnitude_scaling): allow array types for magnitude to a…
lispandfound Feb 3, 2026
b73f0a8
ci: add lock file for reproducibility
lispandfound Feb 3, 2026
fd7b25e
fix(magnitude_scaling): fix array type handling
lispandfound Feb 3, 2026
9bf37aa
ci: fix deptry
lispandfound Feb 3, 2026
be67921
ci: add gdal for fiona
lispandfound Feb 3, 2026
b16ed36
fix(magnitude_scaling): add tests and coercion for magnitude scaling
lispandfound Feb 3, 2026
5bd2ece
fix(types): type checking fixes
lispandfound Feb 3, 2026
809ddd5
fix(magnitude_scaling): address PR comments
lispandfound Feb 3, 2026
02eaecd
fix(srf): revert logic change for segment indexing
lispandfound Feb 3, 2026
829b53c
docs(test_magnitude_scaling): add rationale for input type checks
lispandfound Feb 3, 2026
8725e6e
fix(magnitude_scaling): do not cast to int ever!
lispandfound Feb 3, 2026
1a57dfa
docs(test_magnitude_scaling): minor inaccuracy in docstring
lispandfound Feb 3, 2026
c11b536
tests(test_magnitude_scaling): explain the white lie of the return type
lispandfound Feb 3, 2026
564d726
docs(srf): fix return type
lispandfound Feb 3, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion .github/workflows/deptry.yml
View file
Open in desktop
Copy link
Copy Markdown
Contributor Author

@lispandfound lispandfound Feb 3, 2026

Choose a reason for hiding this comment

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

Just moved this config to pyproject.toml.

Original file line number Diff line number Diff line change
Expand Up @@ -15,4 +15,4 @@ jobs:
# Install deptry
- run: uv pip install deptry
# Run deptry to check that all dependencies are present.
- run: uv run deptry . -ddg test,types
- run: uv run deptry .
2 changes: 1 addition & 1 deletion .github/workflows/pytest.yml
View file
Open in desktop
Copy link
Copy Markdown
Contributor Author

@lispandfound lispandfound Feb 3, 2026

Choose a reason for hiding this comment

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

gdal is required for fiona now.

Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ jobs:
with:
python-version: "3.x"
- name: Install GMT
run: sudo apt-get update && sudo apt-get install -y gmt libgmt-dev ghostscript
run: sudo apt-get update && sudo apt-get install -y gmt libgmt-dev ghostscript libgdal-dev gdal-bin

- name: Define a cache dependency glob
uses: astral-sh/setup-uv@v5
Expand Down
5 changes: 1 addition & 4 deletions .github/workflows/types.yml
View file
Open in desktop
Copy link
Copy Markdown
Contributor Author

@lispandfound lispandfound Feb 3, 2026

Choose a reason for hiding this comment

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

uv sync is not necessary, uv run does that by default.

Original file line number Diff line number Diff line change
Expand Up @@ -22,8 +22,5 @@ jobs:
with:
enable-cache: true

- name: Install project with types
run: uv sync --all-extras --dev

- name: Run type checking with ty
run: uv run ty check --exclude source_modelling/ccldpy.py --exclude setup.py
run: uv run --all-extras ty check --exclude source_modelling/ccldpy.py --exclude setup.py
Comment on lines 28 to +26
Copy link

Copilot AI Feb 3, 2026

Choose a reason for hiding this comment

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

In this workflow, uv run --all-extras ty check ... assumes that ty is available, but in pyproject.toml it is only listed in the dev dependency group and not in the main dependencies or types extra. Depending on uv's defaults, this may mean ty is not installed when the job runs; to make the step robust, consider either selecting the dev group explicitly (e.g. with -g dev) or adding a preceding uv sync that includes the dev group.

Copilot uses AI. Check for mistakes.
11 changes: 9 additions & 2 deletions pyproject.toml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ name = "source-modelling"
authors = [{ name = "ucgmsim" }]
description = "Source modelling library"
readme = "README.md"
requires-python = ">=3.11,<3.14"
requires-python = ">=3.11"
dynamic = ["version"]
dependencies = [
"fiona",
Expand Down Expand Up @@ -36,9 +36,16 @@ types = [
"scipy-stubs",
"pandas-stubs",
"types-networkx",
"ty",
]
dev = ["ty", "ruff", "deptry"]

[tool.deptry]
pep621_dev_dependency_groups = ["test", "dev", "types"]


[tool.ty.rules]
# Downgraded to warning because operators -,+,*,/ are too annoying to type properly
Copy link
Copy Markdown
Contributor Author

@lispandfound lispandfound Feb 3, 2026

Choose a reason for hiding this comment

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

A bunch of the subtractions in magnitude scaling module are technically invalid only because of the cursed way numpy operates. It can't really handle the case where you do something like float - array because it uses the float __sub__ which technically only accepts other floats. This returns a not implemented error, and then it uses the array __rsub__ method, which does work. But the type checker doesn't understand this and stops at __sub__ doesn't work.

unsupported-operator = "warn"

[tool.setuptools.package-data]
source_modelling = ['NZ_CFM/*']
Expand Down
Loading
Loading