FIX: evaluate_connect_function should raise error on un-nested imports

[scott-huberty]

Summary

if I'm understanding evaluate_connect_function correctly, the intent of the if-clause is to catch errors that arise from calling libraries/modules/functions that were imported at the module level (as opposed to nesting the imports at the function level), and raises a more informative error for those cases.

I think nipreps/nibabies#365 gives one example where the if-clause should catch such an error, but doesn't, because the error doesn't start with "global name". So I think it should be okay to make the if-clause more permissive, i.e. just check if the error message ends with "is not defined", then return the extra-informative error message to the user.

What do you think?

[effigies]

I think this is probably a change in Python's error messages, which should not be relied upon as an API. We should probably just do:

 except NameError as e: e.args = ( *e.args, "Due to engine constraints all imports have to be done inside each function definition" ) raise e

[codecov]

Codecov Report

Attention: Patch coverage is 0% with 1 line in your changes missing coverage. Please review.

Project coverage is 70.85%. Comparing base (fffc1c2) to head (df241b2).
Report is 94 commits behind head on master.

Files with missing lines	Patch %	Lines
nipype/pipeline/engine/utils.py	0.00%	1 Missing ⚠️

Additional details and impacted files

@@ Coverage Diff @@ ## master #3655 +/- ## ========================================= Coverage 70.85% 70.85% ========================================= Files 1277 1277 Lines 59118 59116 -2 Branches 9803 8596 -1207 ========================================= - Hits 41888 41887 -1  - Misses 16061 16084 +23  + Partials 1169 1145 -24 

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[scott-huberty]

I think this is probably a change in Python's error messages, which should not be relied upon as an API. We should probably just do:

 except NameError as e: e.args = ( *e.args, "Due to engine constraints all imports have to be done inside each function definition" ) raise e

Thanks @effigies , I think that is a nice idea (much simpler).

I incorporated this suggestion in 4e0b503 , except that I just explicitly raise a NameError, and via string formatting I include the original message plus our extra info (as opposed to overwriting the original error message). Since it looks like Nipype supports 3.7 or later I think this should be okay, but let me know if you disagree!