nipype / pydra

Pydra Dataflow Engine
https://nipype.github.io/pydra/
Other
119 stars 57 forks source link

TypeError: issubclass() arg 1 must be a class #741

Closed ghisvail closed 3 months ago

ghisvail commented 3 months ago

More issues discovered whilst trying out Pydra v0.23 with Clinica.

This time it's an issue when running a pipeline containing a Nipype1Task.

File "<input>", line 1, in <module>
  File "/Users/ghislain.vaillant/Library/Caches/pypoetry/virtualenvs/clinica-_Ggo9PP8-py3.12/lib/python3.12/site-packages/pydra/engine/core.py", line 463, in __call__
    res = self._run(rerun=rerun, environment=environment, **kwargs)
          ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/Users/ghislain.vaillant/Library/Caches/pypoetry/virtualenvs/clinica-_Ggo9PP8-py3.12/lib/python3.12/site-packages/pydra/engine/core.py", line 553, in _run
    orig_inputs = self._modify_inputs()
                  ^^^^^^^^^^^^^^^^^^^^^
  File "/Users/ghislain.vaillant/Library/Caches/pypoetry/virtualenvs/clinica-_Ggo9PP8-py3.12/lib/python3.12/site-packages/pydra/engine/core.py", line 489, in _modify_inputs
    if value is not attr.NOTHING and TypeParser.contains_type(
                                     ^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/Users/ghislain.vaillant/Library/Caches/pypoetry/virtualenvs/clinica-_Ggo9PP8-py3.12/lib/python3.12/site-packages/pydra/utils/typing.py", line 724, in contains_type
    if cls.is_subclass(type_, target):
       ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/Users/ghislain.vaillant/Library/Caches/pypoetry/virtualenvs/clinica-_Ggo9PP8-py3.12/lib/python3.12/site-packages/pydra/utils/typing.py", line 709, in is_subclass
    if issubclass(origin if origin else klass, candidate):
       ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "<frozen abc>", line 123, in __subclasscheck__
TypeError: issubclass() arg 1 must be a class

Which is fair, since:

origin: None,  klass: None, candidate: <class 'fileformats.core.fileset.FileSet'>

So the first argument to issubclass is None, so indeed not a class.

effigies commented 3 months ago

Reproducing in https://github.com/nipype/pydra-nipype1/pull/35.

effigies commented 3 months ago

Should be fixed with pydra-nipype1 0.3.0.

ghisvail commented 3 months ago

Looks like the issue is resolved from the pydra-nipype1 side of things.

I am just wondering whether this part of the codebase should be made more robust? Is it logically sound to have both origin and klass set to None, or should we throw an exception with a more helpful message?

Maybe @tclose could give us his opinion on this use case?

tclose commented 3 months ago

Looks like the issue is resolved from the pydra-nipype1 side of things.

I am just wondering whether this part of the codebase should be made more robust? Is it logically sound to have both origin and klass set to None, or should we throw an exception with a more helpful message?

Maybe @tclose could give us his opinion on this use case?

So is the issue that None should be effectively treated as NoneType when used for typing? Seeing as though Python typing allows this, I'm ok with it in principle.

tclose commented 3 months ago

742 PR addresses this issue by permitting None classes to be interpreted as NoneType