Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Missing stub files confuse static type checkers #1385

Open
s-sajid-ali opened this issue Mar 2, 2023 · 7 comments
Open

Missing stub files confuse static type checkers #1385

s-sajid-ali opened this issue Mar 2, 2023 · 7 comments

Comments

@s-sajid-ali
Copy link
Member

s-sajid-ali commented Mar 2, 2023

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 pybind/pybind11#2350, this can be fixed by using pybind11-stubgen/mypy-stubgen/etc to generate the stubs.

Documents of interest

@ax3l ax3l added this to the 0.15.0 milestone Mar 2, 2023
@ax3l
Copy link
Member

ax3l commented Mar 2, 2023

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
Copy link
Member Author

Aren't all the detected items the the objects present here:

from . import openpmd_api_cxx as cxx
from .DaskArray import record_component_to_daskarray
from .DaskDataFrame import particles_to_daskdataframe
from .DataFrame import particles_to_dataframe
from .openpmd_api_cxx import * # noqa
__version__ = cxx.__version__
__doc__ = cxx.__doc__
__license__ = cxx.__license__
# __author__ = cxx.__author__
# extend CXX classes with extra methods
ParticleSpecies.to_df = particles_to_dataframe # noqa
ParticleSpecies.to_dask = particles_to_daskdataframe # noqa
Record_Component.to_dask_array = record_component_to_daskarray # noqa
# TODO remove in future versions (deprecated)
Access_Type = Access # noqa

@ax3l
Copy link
Member

ax3l commented Mar 2, 2023

Is there a way to tell intellisense to follow this line?

from .openpmd_api_cxx import * # noqa

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
Copy link
Member Author

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
Copy link
Member

ax3l commented Mar 2, 2023

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.

@ax3l ax3l modified the milestones: 0.15.0, 0.16.0 Mar 2, 2023
@s-sajid-ali
Copy link
Member Author

One example on how to do so is here pybind/pybind11#2350 (comment), and I haven't kept up with how build systems have been working to provide this functionality.

@ax3l
Copy link
Member

ax3l commented Aug 31, 2023

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

@ax3l ax3l self-assigned this Aug 31, 2023
@ax3l ax3l modified the milestones: 0.16.0, 0.17.0 Jun 11, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants