ENH: Propagate args / kwargs to loadLoadables | ⚠️ Changes removed from main

[marcus-wirtz-snkeos]

⚠️ Changes originally introduced through this pull-request have been removed from main by force pushing. Corresponding changes are superseded by #7330 ⚠️

Rational:

• Commit message was incorrect
• Trailing white spaces were introduced. This is being fixed in:
  • Trim trailing whitespaces and update pre-commit config to include "trailing-whitespace" #7332

---

Would be great to have the progressbar option of loadSeriesByUID also for the higher level functions.

[lassoan]

Thank you for your contribution, I agree that it can be useful to expose messages and progressCallback arguments at higher-level functions, too. I would prefer to explicitly specify these arguments, as it would make the API more clear (and 2 extra arguments are not too many and I don't expect that loadLoadables will have many new options in the future).

[marcus-wirtz-snkeos]

Thank you for your contribution, I agree that it can be useful to expose messages and progressCallback arguments at higher-level functions, too. I would prefer to explicitly specify these arguments, as it would make the API more clear (and 2 extra arguments are not too many and I don't expect that loadLoadables will have many new options in the future).

Alright, I explicitly specified the arguments. I realized that the main time consumer (~80%) in most of my cases is actually the function getLoadablesFromFileLists rather than loadLoadable. Does that match your experience? I updated the PR to pass the progressbar to the longer lasting function call.

[jcfr]

Nitpick: Adding type annotation would be nice as well though this is outside of the immediate scope of this pull request

[marcus-wirtz-snkeos]

Nitpick: Adding type annotation would be nice as well though this is outside of the immediate scope of this pull request

I don't see a single type hint in this entire module so far (even in the entire Scripted folder?) :-)

[jcfr]

We indeed only recently started to introduce them in new code.

For example:

• Modules/Scripted/DICOMLib¹
• Base/Python/slicer/parameterNodeWrapper²

In this case, adding annotation for messages would be sufficient. Adding the type for callback would be more involved.

Footnotes

1. https://github.com/Slicer/Slicer/blob/main/Modules/Scripted/DICOMLib/DICOMProcesses.py#L55-L58 ↩

2. https://github.com/Slicer/Slicer/blob/main/Base/Python/slicer/parameterNodeWrapper/wrapper.py ↩

[jcfr]

Suggested change

	def loadPatientByUID(patientUID, messages=None, progressCallback=None):
	def loadPatientByUID(patientUID, messages: Optional[list[str]]=None, progressCallback=None):

You also need to add the following at the top:

from typing import Optional

[jcfr]

Integrating. Annotation type will be introduced later.