search

SlicerDMRI / whitematteranalysis

White matter tractography clustering and more...
https://dmri.slicer.org/whitematteranalysis/
Other
65 stars 32 forks source link

WMA 4.0 errors with data type #228

Closed JoshuaKening closed 5 months ago

JoshuaKening commented 6 months ago
image

Both python3.10 and 3.11 versions report errors simultaneously. The latest version of the WMA package in the python 3.11 environment, print(f'Could not load polydata file: {polydata}') and the like will get an error, but print("could not load polydata file: ", "{}".format(polyata)) will not get an error. In addition file congeal_multisubject.py line 479 and congeal_to_atlas.py line 111 both need to be adjusted to float.

JoshuaKening commented 6 months ago

the former error was happened in file harden_transform_with_slicer.py image Moreover, the code import slicer, if error happens: ModuleNotFoundError: No module named 'slicer' "pip install slicer" will not solve this problem due to the ambiguity. It may be that SlicerDMRI in 3D Slicer provides the Slicer module， automatic path acquisition is required

jhlegarreta commented 6 months ago

, but print("could not load polydata file: ", "{}".format(polyata)) will not get an error. In addition file [congeal_multisubject.py]

Rolling back to str.format() is not an option: f-strings were introduced in Python in an attempt to solve the issues of other string formatting options. See https://peps.python.org/pep-0498/#rationale to have more context, and https://docs.python.org/3/tutorial/inputoutput.html for some more documentation.

Although I take responsibility of having introduced f-strings in WMA, we would kindly request @JoshuaKening to identify a fix for the issue using f-strings, and to open the corresponding PR.

Edit: also, there might be other appearances that might have been affected, so when addressing the issue, please look for these so that the fix is applied to all at the same time.

jhlegarreta commented 5 months ago

@JoshuaKening I had a look at this:

  1. The issues described belong to very different things, and the descriptions are confusing. This makes things a lot more harder to understand, and require a lot more time from maintainers. Please, have a look at online resources to create meaningful issue reports, and make one issue be related to a single thing.

  2. Report the exact calls to the scripts, including the parameters, and provide data to reproduce the error.

  3. I am able to run wm_register_to_atlas_new.py without issues. I am also able to run wm_register_multisubject_faster.py, which calls to congeal_multisubject. I am using the HEAD commit.

  4. There is nothing wrong with the line https://github.com/SlicerDMRI/whitematteranalysis/blob/165cfcdb321adf7177938d7c2f6723500cc4c9eb/whitematteranalysis/congeal_to_atlas.py#L166

    If you execute it giving arbitrary integer values to total_iterations and sigma you will see that it works flawlessly. The issue that is shown in your screenshot appears when any of those values are not integers. I ignore how they can be set to floats, because they are not exposed if I read the code correctly.

    If I use the .format() syntax no error is raised even if one the values is an integer: https://github.com/SlicerDMRI/whitematteranalysis/blame/0cb54a8cc8c958ca75e4f3608ae55c44ca9524ab/whitematteranalysis/congeal_to_atlas.py#L166C23-L166C88

    If you change the line to (ignore the removal of the self. calls)

    dirname = f"iteration_{total_iterations:05.0f}_sigma_{sigma:05.0f}"

    no error is raised and the result is the expected one.

    But the underlying question is how those values can get negative values: I see no assignment statement that would use float values on the file at issue.

  5. , the code import slicer, if error happens: ModuleNotFoundError: No module named 'slicer'

    Maybe the script was only intended to be used from https://github.com/SlicerDMRI/SlicerWMA. I ignore this. A pip install slicer will not work because no up-to-date packages exist on PyPI: https://pypi.org/project/slicer/

    Check the Python version and the dates on PyPI. They speak for themselves.

  6. As for the print(f'Could not load polydata file: {polydata}') issue, assuming the code can reach that point, I do not see where the error stems from: polydata is assumed to be a string, sot the above should run without issues: if you take that very statement and run it interactively with an arbitrary string you will see that it has no issues.

All the above could have been discovered debugging WMA. There is no complex setting to debug WMA, you are kindly requested to invest that time to fix issues that you may encounter. I am closing the issue for now.