flux-framework / flux-core

core services for the Flux resource management framework
GNU Lesser General Public License v3.0
159 stars 49 forks source link

Python: `flux.job.wait` is overloaded #5487

Open jameshcorbett opened 9 months ago

jameshcorbett commented 9 months ago

@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.

grondo commented 9 months ago

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

wihobbs commented 9 months ago

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.

jameshcorbett commented 9 months ago

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.

wihobbs commented 8 months ago

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 commented 8 months ago

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.