flux-core icon indicating copy to clipboard operation
flux-core copied to clipboard

Python: `flux.job.wait` is overloaded

Open jameshcorbett opened this issue 2 years ago • 6 comments
trafficstars

@wihobbs was following the Python documentation, where functions are listed by their full path. For instance, the function which most Flux developers know as flux.job.wait is listed as flux.job.wait.wait, because actually that function is defined in flux/job/wait.py.

Hobbs found that he couldn't use the flux.job.wait module, and when I looked into it I found that it's because flux/job/__init__.py has the line from flux.job.wait import wait_async, wait, wait_get_status, result_async, result. This makes the variable flux.job.wait refer to the function flux.job.wait.wait rather than the module flux.job.wait.

>>> import flux.job.wait

>>> flux.job.wait.result
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
AttributeError: 'function' object has no attribute 'result'
>>> type(flux.job.wait)
<class 'function'>

This means that the best way to refer to the flux.job.wait module and its functions is sys.modules["flux.job.wait"].

See also https://stackoverflow.com/questions/22374155/python-importing-a-module-with-the-same-name-as-a-function

To allow users to refer to functions as they are listed in the documentation, I think we would need to rename the module, or rename the function, or drop from flux.job.wait import wait from the __init__.py.

But we could also fix the documentation to list the flux.job.wait.wait function as flux.job.wait instead. Which would probably be better.

This is a more general problem than just wait, if affects other modules such as submit and kill.

jameshcorbett avatar Oct 05 '23 18:10 jameshcorbett

Good catch, are the functions used as flux.job.wait.wait in the docs because the documentation is autogenerated?

grondo avatar Oct 05 '23 18:10 grondo

Note that some (but not all) of the flux.job.wait functions are documented (with working function calls) under the Asynchronous event loop monitoring and submission documentation.

wihobbs avatar Oct 05 '23 19:10 wihobbs

Good catch, are the functions used as flux.job.wait.wait in the docs because the documentation is autogenerated?

I think so. It could be there's a way to fix the autogeneration though.

jameshcorbett avatar Oct 05 '23 19:10 jameshcorbett

This might be a dumb idea, but what if we just renamed wait to something else, i.e. flux.job.sync.wait? This wouldn't break any functionality if the wait call was still loaded in the flux/job/__init__.py, and it wouldn't be overloaded, because it would have a different name. It might require a few internal changes but external user functionality and calls would remain the same. However, a docs only fix (if one exists) is perfectly reasonable too.

wihobbs avatar Oct 13 '23 08:10 wihobbs

I think we would need to rename the module

Whelp, should've gone back and read this before I suggested it. Anyway, I'll look and see if a docs only fix is available.

wihobbs avatar Oct 13 '23 08:10 wihobbs