Fix type annotations for array inputs in signal functions

[Spkap]

This PR fixes the type annotations in signal_linear() and signal_SPGR() functions to properly reflect that they handle both scalar and array inputs.

The fix uses NDArray[np.float64] to annotate the parameters and return values, which correctly handles both NumPy scalars (0D arrays) and multi-dimensional arrays.

Fixes #61

[Spkap]

@ltorres6, this PR addresses issue #61. Please review it and let me know if you have any feedback. Thank you!

[ltorres6]

Sorry for the confusion. There are some paramters that will likely always remain scalars, and should not be arrays. TR, k, and a should be scalars.

Additionally, would you mind ensuring we accept both float32 and float64?

[Spkap]

I've used np.floating to support both float32 and float64 instead of union types. Is this approach optimal, or would you prefer a different implementation?

[plaresmedima]

I think its important to have an overarching and consistent strategy to typing for the package as a whole rather than decide this on a function by function basis. It will be very confusing if different functions have different approaches to typing as they are intended to be chained together For instance both these functions will fail if R1 is entered as a list. Is that the intended behaviour? For a numpy user (i.e. practically any scientific python user) this is contra-intuitive - is there any good reason why this would fail if the user enters R1 as a list? Is the result in some way ambiguous? Once there is a conistent approach to typing you can then decide how types are to be encoded in the documentation and function definitions. But that is a secondary consideration. Another point is type checking - if some types of input are forbidden (i.e. a list in these examples) you need to check for it and return an informative error message.

…

On Mon, 17 Mar 2025, 21:45 Sourabh Kapure, ***@***.***> wrote: I've used np.floating to support both float32 and float64 instead of union types. Is this approach optimal, or would you prefer a different implementation? — Reply to this email directly, view it on GitHub <#63 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/ABOFKA2LK44PRBYCDW2EDJL2U466JAVCNFSM6AAAAABZFQDFPOVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDOMZRGAYDKMZWGA> . You are receiving this because you are subscribed to this thread.Message ID: ***@***.***> [image: Spkap]*Spkap* left a comment (OSIPI/osipi#63) <#63 (comment)> I've used np.floating to support both float32 and float64 instead of union types. Is this approach optimal, or would you prefer a different implementation? — Reply to this email directly, view it on GitHub <#63 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/ABOFKA2LK44PRBYCDW2EDJL2U466JAVCNFSM6AAAAABZFQDFPOVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDOMZRGAYDKMZWGA> . You are receiving this because you are subscribed to this thread.Message ID: ***@***.***>

[Spkap]

@plaresmedima Thank you for your suggestions. I completely agree that we need a consistent package-wide typing approach rather than function-by-function decisions.

You raise an excellent point about lists currently the annotations would reject them when NumPy users would expect them to work. We should follow NumPy's pattern of accepting array-like objects (lists, tuples, etc.) and converting them internally.

For a consistent approach, I see two main options:

1. Use NumPy's built-in ArrayLike type and convert inputs with np.asarray():

   from numpy.typing import ArrayLike
   
   def signal_linear(R1: ArrayLike, k: float) -> np.ndarray:
       R1 = np.asarray(R1)
       return k * R1

2. Create input validation functions with informative error messages that enforce consistent behavior across the package.

What are your thoughts on these approaches? Alternatively, please feel free to suggest any ideas you might have.

[Spkap]

@ltorres6 I've updated the PR accordingly, now we support both float32 and float64 data types. Please review it and let me know if you have any feedback. Thank you!

[plaresmedima]

I think your solution is perfect - thanks a lot for considering my comment so carefully. I didn't actually know numpy had a formal ArrayLike type or a numpy.typing module. Thanks for pointing that out! Prof. Steven Sourbron, PhD Chair in Medical Imaging Physics University of Sheffield, UK *https <https://www.sheffield.ac.uk/medicine/people/iicd/steven-sourbron>://www.sheffield.ac.uk/medicine/people/iicd/steven-sourbron <https://www.sheffield.ac.uk/medicine/people/iicd/steven-sourbron>* *Sheffield abdominal imaging research page* <https://www.sheffield.ac.uk/medicine/research/research-themes/medical-imaging/medical-imaging-research/abdominal-imaging> *Open source initiative in perfusion imaging* <http://www.osipi.org> *Magnetic resonance imaging biomarkers for chronic kidney disease <https://renalmri.org/>*

…

On Wed, 19 Mar 2025 at 13:47, Sourabh Kapure ***@***.***> wrote: @plaresmedima <https://github.com/plaresmedima> Thank you for your suggestions. I completely agree that we need a consistent package-wide typing approach rather than function-by-function decisions. You raise an excellent point about lists currently the annotations would reject them when NumPy users would expect them to work. We should follow NumPy's pattern of accepting array-like objects (lists, tuples, etc.) and converting them internally. For a consistent approach, I see two main options: 1. Use NumPy's built-in ArrayLike type and convert inputs with np.asarray(): from numpy.typing import ArrayLike def signal_linear(R1: ArrayLike, k: float) -> np.ndarray: R1 = np.asarray(R1) return k * R1 2. Create input validation functions with informative error messages that enforce consistent behavior across the package. What are your thoughts on these approaches? Alternatively, please feel free to suggest any ideas you might have. — Reply to this email directly, view it on GitHub <#63 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/ABOFKA6OI2JR6G6M2OFW4GD2VFYOVAVCNFSM6AAAAABZFQDFPOVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDOMZWG4YTAOBSGQ> . You are receiving this because you were mentioned.Message ID: ***@***.***> [image: Spkap]*Spkap* left a comment (OSIPI/osipi#63) <#63 (comment)> @plaresmedima <https://github.com/plaresmedima> Thank you for your suggestions. I completely agree that we need a consistent package-wide typing approach rather than function-by-function decisions. You raise an excellent point about lists currently the annotations would reject them when NumPy users would expect them to work. We should follow NumPy's pattern of accepting array-like objects (lists, tuples, etc.) and converting them internally. For a consistent approach, I see two main options: 1. Use NumPy's built-in ArrayLike type and convert inputs with np.asarray(): from numpy.typing import ArrayLike def signal_linear(R1: ArrayLike, k: float) -> np.ndarray: R1 = np.asarray(R1) return k * R1 2. Create input validation functions with informative error messages that enforce consistent behavior across the package. What are your thoughts on these approaches? Alternatively, please feel free to suggest any ideas you might have. — Reply to this email directly, view it on GitHub <#63 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/ABOFKA6OI2JR6G6M2OFW4GD2VFYOVAVCNFSM6AAAAABZFQDFPOVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDOMZWG4YTAOBSGQ> . You are receiving this because you were mentioned.Message ID: ***@***.***>

[Spkap]

@ltorres6, just following up to see if the latest changes align with your expectations. The PR now supports both float32 and float64 while ensuring that TR, k, and a remain scalars.

When you have a moment, could you please review the changes and let me know if anything else needs adjustment? Thanks!