openPMD / openPMD-api

:floppy_disk: C++ & Python API for Scientific I/O
https://openpmd-api.readthedocs.io
GNU Lesser General Public License v3.0
142 stars 51 forks source link

Missing stub files confuse static type checkers #1385

Open s-sajid-ali opened 1 year ago

s-sajid-ali commented 1 year ago

Currently, the openpmd-api module confuses static type checkers like pyright:

Screen Shot 2023-03-02 at 2 02 46 PM

Inspecting the openpmd-api module via inspect and dir using the following:

sasyed@MAC-140753 ~/D/p/t/heponhpc> cat tmp.py
import openpmd_api as io
import inspect

print("is openpmd-api a module? ", inspect.ismodule(io))

print("what methods are available via openpmd-api? \n", dir(io))

I see

sasyed@MAC-140753 ~/D/p/t/heponhpc> python tmp.py                                                                                                                       
is openpmd-api a module?  True
what methods are available via openpmd-api? 
 ['Access', 'Access_Type', 'Allocation', 'Attributable', 'Attribute_Keys', 'Base_Record_Base_Record_Component', 
 'Base_Record_Component', 'Base_Record_Component_Container', 'Base_Record_Mesh_Record_Component', 
 'Base_Record_Patch_Record_Component', 'Base_Record_Record_Component', 'ChunkInfo', 'DaskArray', 
 'DaskDataFrame', 'DataFrame', 'Dataset', 'Datatype', 'Dynamic_Memory_View', 'Geometry', 'IndexedIteration', 
 'Iteration', 'Iteration_Container', 'Iteration_Encoding', 'Mesh', 'Mesh_Container', 'Mesh_Record_Component', 
 'Mesh_Record_Component_Container', 'ParticleSpecies', 'Particle_Container', 'Particle_Patches', 
 'Particle_Patches_Container', 'Patch_Record', 'Patch_Record_Component', 
 'Patch_Record_Component_Container', 'Patch_Record_Container', 'ReadIterations', 'Record', 
 'Record_Component', 'Record_Component_Container', 'Record_Container', 'Series', 'Unit_Dimension', 
 'WriteIterations', 'WrittenChunkInfo', '__builtins__', '__cached__', '__doc__', '__file__', '__license__', 
 '__loader__', '__name__', '__package__', '__path__', '__spec__', '__version__', 'cxx', 
 'determine_datatype', 'file_extensions', 'list_series', 'openpmd_api_cxx', '
 particles_to_daskdataframe', 'particles_to_dataframe', 'record_component_to_daskarray', 
 'variants']
sasyed@MAC-140753 ~/D/p/t/heponhpc>    

However, if I use the LSP command to go to the definition of the openpmd-api module, I am directed to this __init__.py in the install directory which is missing the definition for Series.

Proposed fix

Per https://github.com/pybind/pybind11/issues/2350, this can be fixed by using pybind11-stubgen/mypy-stubgen/etc to generate the stubs.

Documents of interest

ax3l commented 1 year ago

Thanks for finding and reporting this!

It might be that this can be simpler and we just need to add some __repr__/__str__ and doc strings of similar kinds to Series. It is otherwise imported like all the other classes you saw in your screenshot on Slack. Uploading here so we see what it already detects: Screen Shot 2023-02-13 at 12 53 42 PM

s-sajid-ali commented 1 year ago

Aren't all the detected items the the objects present here: https://github.com/openPMD/openPMD-api/blob/9a215b20b48abc9275ef53306cfd5e357bf86ed5/src/binding/python/openpmd_api/__init__.py#L1-L18

ax3l commented 1 year ago

Is there a way to tell intellisense to follow this line? https://github.com/openPMD/openPMD-api/blob/9a215b20b48abc9275ef53306cfd5e357bf86ed5/src/binding/python/openpmd_api/__init__.py#L5

Maybe it generally ignores that.

Otherwise we could consider looping over the imported dicts and adding them to __all__ or so, but that's a bit much for a functionally correct workflow that confuses an IDE

s-sajid-ali commented 1 year ago

No sure about intellesense but a different static type checker, mypy states:

Mypy can use a stub file instead of the real implementation to provide type information for the module. They
are useful for third-party modules whose authors have not yet added type hints (and when no stubs are 
available in typeshed) and C extension modules (which mypy can’t directly process).

which makes me guess that the answer to your question is no, though I'd have to read more about how pyright or intellesense works to be sure.

ax3l commented 1 year ago

Hm, I see, they don't query the extension module at all? Why not I wonder, it's available like any other class... So we need stubs I guess and a workflow to not have to create them manually all the time we change the bindings.

s-sajid-ali commented 1 year ago

One example on how to do so is here https://github.com/pybind/pybind11/issues/2350#issuecomment-1126255196, and I haven't kept up with how build systems have been working to provide this functionality.

ax3l commented 1 year ago

I started working on this in pyAMReX and then will propagate the solution to here, too. https://github.com/AMReX-Codes/pyamrex/pull/184