Skip to content
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.

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

STYLE: Fix registration process id string floating point formatting #236

Merged

Conversation

jhlegarreta
Copy link
Contributor

@jhlegarreta jhlegarreta commented Jul 6, 2024

Fix formatting of numbers that can be floats when creating the registration process id string: when transitioning to f-strings (commit ac6040f), it was assumed that sigma and grid_resolution would be integers, based on the existing formatting. However, these attributes can be set to non-integer values. The pre-f-string transition solution, e.g.:

"(...)_sigma_%03d_grid_%03d" % (subject_idx, iteration_count, sigma, grid_resolution)

trimmed the decimal part, and thus, for e.g. sigma 7.5 and grid_resolution 1.5, it would produce:

'(...)_sigma_007_grid_001'

With the introduction of f-strings, i.e.

f"(...)_sigma_{sigma:03d}_grid_{grid_resolution:03d}"

this would produce an error, since the sigma and grid_resolution floating point value cannot be formatted as an integer:

{ValueError}ValueError("Unknown format code 'd' for object of type
'float'")

This patch set fixes the error by adopting the previous behavior: it trims the decimal part.

Left behind in commit d7c6bd8.

Limit the Pillow dependency to <10.4.0. Solves:

  File "/opt/hostedtoolcache/Python/3.8.18/x64/lib/python3.8/site-packages/PIL/_typing.py", line 10, in <module>
    NumpyArray = npt.NDArray[Any]
AttributeError: module 'numpy.typing' has no attribute 'NDArray'

raised for example at:
https://github.com/SlicerDMRI/whitematteranalysis/actions/runs/9821374452/job/27116961065?pr=236#step:6:175

@jhlegarreta jhlegarreta changed the title iSTYLE: Fix registration id string creation floating point formatting STYLE: Fix registration id string creation floating point formatting Jul 6, 2024
@jhlegarreta jhlegarreta force-pushed the FixFloatingPointFormatting2 branch 2 times, most recently from ff6648f to cc762c6 Compare July 6, 2024 18:55
Fix formatting of numbers that can be floats when creating the
registration process id string: when transitioning to f-strings (commit
ac6040f), it was assumed that `sigma` and `grid_resolution` would be
integers, based on the existing formatting. However, these attributes
can be set to non-integer values. The pre-f-string transition solution, e.g.:
```
"(...)_sigma_%03d_grid_%03d" % (subject_idx, iteration_count, sigma, grid_resolution)
```

trimmed the decimal part, and thus, for e.g. `sigma` 7.5 and
`grid_resolution` 1.5, it would produce:
```
'(...)_sigma_007_grid_001'
```

With the introduction of f-strings, i.e.
```
f"(...)_sigma_{sigma:03d}_grid_{grid_resolution:03d}"
```

this would produce an error, since the `sigma` and `grid_resolution`
floating point value cannot be formatted as an integer:
```
{ValueError}ValueError("Unknown format code 'd' for object of type
'float'")
```

This patch set fixes the error by adopting the previous behavior: it
trims the decimal part.

Left behind in commit d7c6bd8.

Limit the Pillow dependency to <10.4.0. Solves:
```
  File "/opt/hostedtoolcache/Python/3.8.18/x64/lib/python3.8/site-packages/PIL/_typing.py", line 10, in <module>
    NumpyArray = npt.NDArray[Any]
AttributeError: module 'numpy.typing' has no attribute 'NDArray'
```

raised for example at:
https://github.com/SlicerDMRI/whitematteranalysis/actions/runs/9821374452/job/27116961065?pr=236#step:6:175
@jhlegarreta jhlegarreta changed the title STYLE: Fix registration id string creation floating point formatting STYLE: Fix registration process id string floating point formatting Jul 6, 2024
@jhlegarreta jhlegarreta merged commit 8064245 into SlicerDMRI:master Jul 6, 2024
9 checks passed
@jhlegarreta jhlegarreta deleted the FixFloatingPointFormatting2 branch July 6, 2024 18:59
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.

None yet

1 participant