Improve typing consistency in codebase by ubdbra001 · Pull Request #1311 · AFM-SPM/TopoStats

ubdbra001 · 2026-03-05T15:05:10Z

This PR addresses the inconsistent typing in the codebase.
Given the size of this I'm going to do it incrementally in this PR. The initial stage of this is picking out the "easy wins" e.g. cases where the type hints are missing or incorrect and are easy/obvious to update.

At this stage all the tests still pass.

Before submitting a Pull Request please check the following.

Existing tests pass.
Documentation has been updated and builds. Remember to update as required...
- docs/configuration.md
- docs/usage.md
- docs/data_dictionary.md
- docs/advanced.md and new pages it should link to.
Pre-commit checks pass.

codecov · 2026-03-05T15:15:22Z

Codecov Report

❌ Patch coverage is 91.48936% with 4 lines in your changes missing coverage. Please review.
✅ Project coverage is 84.11%. Comparing base (6c2ef50) to head (86492cf).
⚠️ Report is 110 commits behind head on main.

Files with missing lines	Patch %	Lines
topostats/processing.py	33.33%	4 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #1311      +/-   ##
==========================================
- Coverage   87.98%   84.11%   -3.88%     
==========================================
  Files          31       31              
  Lines        6027     6447     +420     
==========================================
+ Hits         5303     5423     +120     
- Misses        724     1024     +300

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

ns-rse

Good work tackling this behemoth of a task @ubdbra001 and apologies for it being so messy (typing only came onto my radar way after I'd started).

Some comments in-line from a quick scan, don't have time to investigate more deeply I'm afraid.

ns-rse · 2026-03-05T17:17:22Z

            grain_stats_df.index.set_names(["grain_number", "class", "subgrain"], inplace=True)
        else:
            grain_stats_df = None
        return topostats_object.filename, topostats_object, grain_stats_df


Not for this work but I thought I'd removed returning of dataframes as they are instead pulled out of the topostats_objects and collated into a dictionary before converting to pd.DataFrame.

This was the only one I've spotted so far, happy to open an issue to be addressed later

Note it as an issue, its at least then recorded and can be addressed if anyone has time/inclination.

SylviaWhittle

Just a thing I noticed skim-reading this.

ubdbra001 · 2026-03-05T19:14:26Z

@@ -214,7 +214,7 @@ def compile_images(
    @staticmethod
    def remove_common_values(
        ordered_array: npt.NDArray, common_value_check_array: npt.NDArray, retain: list = ()


On this:

retain is typed as list type but being assigned an empty tuple by default 🤔

Setting the default value to an empty list may cause issues (see here).

I'll probably set it as None and add a check so that it get re-set to an empty list if the arg is not supplied.
Any objections?

This is a good solution and yeah, default args should not be data structures as you point out.

ubdbra001 · 2026-03-05T19:17:11Z

+    molecule_data : dict[int, Molecule], optional
        Dictionary of ``Molecule`` objects indexed by molecule number.
-    tracing_stats : dict | None
+    tracing_stats : dict, optional


I have been going around replacing | None with , optional in the docstrings. Probably doesn't have a real impact but I thought I'd make it all consistent.

Let me know if you'd prefer to retain | None

ubdbra001 · 2026-03-05T19:17:46Z

            Dictionary, indexed by molecule where the value is the molecules statistics for the given molecule.
        """
+        if self.molecule_data is None:
+            raise ValueError("No molecule data found")


Let me know if you'd like me to change this to something else

ns-rse

Some responses in-line...

ns-rse · 2026-03-06T11:19:06Z

            grain_stats_df.index.set_names(["grain_number", "class", "subgrain"], inplace=True)
        else:
            grain_stats_df = None
        return topostats_object.filename, topostats_object, grain_stats_df


Note it as an issue, its at least then recorded and can be addressed if anyone has time/inclination.

ns-rse · 2026-03-06T18:27:30Z

Just a thought but it might be worth adding commits to .git-blame-ignore-revs so that the "blame" resides with the original author rather than yourself @ubdbra001.

ubdbra001 · 2026-04-01T11:11:35Z

I think I've got all the low hanging typing fruit, anything else this will cause tests to fail and so I suspect will require a bit more discussion.

resolves typing error for later "molecule_data[i] = ..." as NoneType is not sub-scriptable

Raises valueError if molecule_data is None

Adds correct return typehint Co-authored-by: Neil Shephard <n.shephard@sheffield.ac.uk>

Add correct return type hint Co-authored-by: Sylvia Whittle <86117496+SylviaWhittle@users.noreply.github.com>

Correct to a tuple of str and dict

Corrects the type hint and the docstring

Docstring for second value in return tuple needs to be updated

These specify that the values in the dict are not None after processing

However, I don' see these being used anywhere, so it may be worth just deleting?

To match return value type hint

Currently very broad value type, I couldn't parse what they could be from the code

SylviaWhittle · 2026-05-28T10:49:45Z

@@ -214,7 +214,7 @@ def compile_images(
    @staticmethod
    def remove_common_values(
        ordered_array: npt.NDArray, common_value_check_array: npt.NDArray, retain: list = ()


This is a good solution and yeah, default args should not be data structures as you point out.

SylviaWhittle · 2026-05-28T10:51:54Z

Looked at all changes and have provided answers to queries for docstrings.

This is good to go once the docstrings are updated and provided tests pass.

Thank you very much @ubdbra001.

ubdbra001 changed the title ~~I1299 improve typing consistency~~ Improve typing consistency in codebase Mar 5, 2026

ns-rse reviewed Mar 5, 2026

View reviewed changes

SylviaWhittle reviewed Mar 5, 2026

View reviewed changes

Comment thread topostats/processing.py Outdated

Comment thread topostats/processing.py Outdated

Comment thread topostats/processing.py Outdated

ubdbra001 commented Mar 5, 2026

View reviewed changes

ns-rse reviewed Mar 6, 2026

View reviewed changes

ns-rse mentioned this pull request Mar 12, 2026

[typing] Validate all calls using pydantic.validate_call() decorator #1314

Open

ubdbra001 marked this pull request as ready for review April 1, 2026 11:10

ubdbra001 requested review from SylviaWhittle, ns-rse and tobyallwood April 1, 2026 11:11

ubdbra001 and others added 15 commits May 27, 2026 10:52

update doctring typehints for MatchedBranch

9b22f18

update type hints for UnMatchedBranch

da0dd7d

set molecule_data outside then inside if-statement

676b9e9

resolves typing error for later "molecule_data[i] = ..." as NoneType is not sub-scriptable

correct return type hint for remove_common_values

9cd75f9

updating OrderedTrace doctsrings

aed52e7

add error to collate_molecule_statistics

1cb93c1

Raises valueError if molecule_data is None

update str dunder for when images are None

1398614

update type hints for GrainCrop image get & set

d27c270

update return type hint for calculate_region_connection_regions

67da943

Set directories to Path types in process_scan

9e66b26

update param type and dir type in process_filters

05f8e38

update param type and dir type in process_grains

d403998

update return type and dir type in process_grainstats

e9248fa

[pre-commit.ci] Fixing issues with pre-commit

7e447bf

Update run_nodestats_tracing docstring

2b25eb4

Adds correct return typehint Co-authored-by: Neil Shephard <n.shephard@sheffield.ac.uk>

ubdbra001 and others added 16 commits May 27, 2026 10:52

Update docstring for process_grainstats

67baf5f

Add correct return type hint Co-authored-by: Sylvia Whittle <86117496+SylviaWhittle@users.noreply.github.com>

Update return description for process_grainstats docstring

1333866

Update return typehint for order_from_end

a8c39f2

Correct to a tuple of str and dict

Update identify_writhes return type hints

305ac95

Corrects the type hint and the docstring

Update param type hints in Filters class and methods

eb68419

[pre-commit.ci] Fixing issues with pre-commit

879fc64

update typehints in theme module

9fa544c

update return type hint for remove_scars

480363e

Docstring for second value in return tuple needs to be updated

add inline typehints for image dict values

2bec3a7

These specify that the values in the dict are not None after processing

update typing for bound_padded_coordinates_to_image and check functions

22951a3

However, I don' see these being used anywhere, so it may be worth just deleting?

update type hints for threshold functions

ab6ebcb

convert return val of triangle height to float

15b5d60

To match return value type hint

update return type hints for min_max_feret and get_feret_from_mask

0fbbef8

Currently very broad value type, I couldn't parse what they could be from the code

[pre-commit.ci] Fixing issues with pre-commit

09a4faa

use NDArray not ArrayLike

66e74f1

update doctrings for topostats_objects input arguments

f627bd9

ubdbra001 force-pushed the i1299-improve-typing-consistency branch from 9328755 to f627bd9 Compare May 27, 2026 10:05

Merge branch 'main' into i1299-improve-typing-consistency

dc4a239

SylviaWhittle requested changes May 28, 2026

View reviewed changes

ubdbra001 and others added 5 commits June 9, 2026 09:27

update remove_common_values input args

88895ef

updates to remove_scars

2134a1f

update return docstrings for feret functions

a9bedc2

address sylvia's feedback

9db4b99

[pre-commit.ci] Fixing issues with pre-commit

86492cf

SylviaWhittle merged commit d07b4e5 into AFM-SPM:main Jun 17, 2026
3 of 7 checks passed

ns-rse mentioned this pull request Jun 17, 2026

Apply typehints consistently across the code base #516

Open

Uh oh!

Conversation

ubdbra001 commented Mar 5, 2026

Uh oh!

codecov Bot commented Mar 5, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

ns-rse left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

SylviaWhittle left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

ubdbra001 Mar 5, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ns-rse left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ns-rse commented Mar 6, 2026

Uh oh!

ubdbra001 commented Apr 1, 2026

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

SylviaWhittle commented May 28, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

codecov Bot commented Mar 5, 2026 •

edited

Loading

ubdbra001 Mar 5, 2026 •

edited

Loading

SylviaWhittle commented May 28, 2026 •

edited

Loading