Skip to content

STYLE: Use itk.int64_t and itk.uint64_t instead of if os.name == "nt" - #6767

Open
N-Dekker wants to merge 1 commit into
InsightSoftwareConsortium:mainfrom
N-Dekker:Use-int64_t-instead-of-if-os-name
Open

STYLE: Use itk.int64_t and itk.uint64_t instead of if os.name == "nt"#6767
N-Dekker wants to merge 1 commit into
InsightSoftwareConsortium:mainfrom
N-Dekker:Use-int64_t-instead-of-if-os-name

Conversation

@N-Dekker

Copy link
Copy Markdown
Contributor

Replaced if os.name == "nt" statements which used itk.SL, itk.SLL, itk.UL, or itk.ULL with the equivalent code, using just itk.int64_t or itk.uint64_t.

Aims to improve code readability and remove OS-specific code.

Replaced `if os.name == "nt"` statements which used `itk.SL`, `itk.SLL`,
`itk.UL`, or `itk.ULL` with the equivalent code, using just `itk.int64_t` or
`itk.uint64_t`.

Aims to improve code readability and remove OS-specific code.

Follow-up to pull request InsightSoftwareConsortium#6762
commit dc0ea28
"ENH: Add CType aliases for numeric types of specific sizes to Python"
@github-actions github-actions Bot added area:Python wrapping Python bindings for a class type:Testing Ensure that the purpose of a class is met/the results on a wide set of test cases are correct area:Core Issues affecting the Core module type:Style Style changes: no logic impact (indentation, comments, naming) labels Aug 19, 2026

@hjmjohnson hjmjohnson left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Thanks! I think this is much cleaner.

@N-Dekker

Copy link
Copy Markdown
Contributor Author

Does anyone here know, is this a similar case?

# For windows use itk.ULL
if hasattr(itk.VectorContainer, "ULAD"):
IdentifierType = itk.UL
else:
IdentifierType = itk.ULL

What is the meaning of "ULAD"? And then, is if hasattr(itk.VectorContainer, "ULAD") equivalent to if os.name == "nt"? Would it allow replacing these five lines with a simple IdentifierType = itk.uint64_t as well?

(It's not a show-stopper to me, I'm just wondering... 🤔)

@dzenanz

dzenanz commented Aug 19, 2026

Copy link
Copy Markdown
Member

is if hasattr(itk.VectorContainer, "ULAD") equivalent to if os.name == "nt"

Probably, but I am not entirely sure. PR good even as-is.

@N-Dekker

N-Dekker commented Aug 19, 2026

Copy link
Copy Markdown
Contributor Author

Probably, but I am not entirely sure. PR good even as-is.

Thanks @dzenanz I see now, the "ULAD check" was introduced by Pranjal (@PranjalSahu) PR #2970 commit 7e59d1b, merged on Dec 17, 2021. You already commented at #2970 (comment)

A generic way to check for Windows is if os.name == 'nt'. But if the one you wrote here works, we should keep it.

On my Windows 11 laptop, os.name == 'nt' is true, and hasattr(itk.VectorContainer, "ULAD") is false👍


So in practice, I think hasattr(itk.VectorContainer, "ULAD") is equal to os.name != 'nt' 😃

@N-Dekker
N-Dekker marked this pull request as ready for review August 19, 2026 16:11
@greptile-apps

greptile-apps Bot commented Aug 19, 2026

Copy link
Copy Markdown
Contributor

Greptile Summary

This change replaces platform-name branching with ITK fixed-width integer aliases in Python wrapper support and serialization tests. A parent-versus-update compatibility check exercised NumPy uint64 mapping, vector-container index selection, and the ST, IT, and OT aliases under simulated POSIX and Windows wrapper conventions. The check produced identical 64-bit type selections before and after the change, disproving a regression in those paths. No defects were found.

Confidence Score: 5/5

Safe to merge based on the exercised compatibility paths: the updated aliases preserve the previous 64-bit wrapper behavior on both simulated platform conventions.

The focused parent-versus-update check passed for every changed type-selection path and found no publishable defect.

Files Needing Attention: No files need follow-up attention.

T-Rex T-Rex Logs

What T-Rex did

  • Ran the uint64 alias compatibility check scripts for HEAD^ and HEAD to validate cross-version 64-bit alias handling.
  • The harness loaded the parent and head definitions with minimal ITK and NumPy stubs and exercised NumPy uint64 mapping, vector-container binding selection, and the ST/IT/OT aliases under simulated POSIX and Windows conventions.
  • The combined parent-versus-head run produced PASS parent_and_head_match_for_all_claimed_paths and exit code 0, with both revisions selecting the same 64-bit types.

View all artifacts

T-Rex Ran code and verified through T-Rex

Reviews (1): Last reviewed commit: "STYLE: Use itk.int64_t and itk.uint64_t ..." | Re-trigger Greptile

@dzenanz

dzenanz commented Aug 19, 2026

Copy link
Copy Markdown
Member

It is up to you whether you also want to update the "ULAD check".

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area:Core Issues affecting the Core module area:Python wrapping Python bindings for a class type:Style Style changes: no logic impact (indentation, comments, naming) type:Testing Ensure that the purpose of a class is met/the results on a wide set of test cases are correct

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants