Skip to content

Typing information for pyosmium #59

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

Closed
wants to merge 19 commits into from

Conversation

wiktorn
Copy link
Contributor

@wiktorn wiktorn commented May 6, 2018

This provides *,pyi files with type annotations. As pyosmium is still supporting Python 2.7 I decided to implement type annotations as separate files.

I found this quite useful and informative when using with PyCharm.

For now, I decided to ship *.pyi files together with *.py files. Python community is working on target solution

@lonvia
Copy link
Member

lonvia commented May 21, 2018

I'm not entirely happy with all the code duplication. Keeping the py and pyi files in sync will be a pain. So I'm not sure how to go forward with this. I'm not against adding typing information but it needs to be maintainable.

@wiktorn
Copy link
Contributor Author

wiktorn commented May 21, 2018

I'm not happy too. Though:

  • it's easiest way to maintain compatibility with wide range of python versions
  • it makes easy to annotate pure C(++) functions that doesn't have python representation

We can also try with this approach:
https://mypy.readthedocs.io/en/latest/python2.html#type-checking-python-2-code

But then we need to add typing module backport as runtime requirement.

Then we would need to have stub methods for pure C++ functions.

@wiktorn
Copy link
Contributor Author

wiktorn commented May 21, 2018

I've managed to inline all annotations to *py files, where they do exists:
https://github.com/osmcode/pyosmium/compare/master...wiktorn:typing_annotations_inline?expand=1

For all pure C++ modules I kept the *pyi files.

Does it improve situation to manageable level?

@wiktorn wiktorn force-pushed the typing_annotations branch from 646c6b1 to 4978558 Compare May 22, 2018 19:06
@wiktorn
Copy link
Contributor Author

wiktorn commented Oct 25, 2018

To follow up on this. I was able to use mypy to verify the completeness of the annotations but only for some definitions of completeness, i.e. only if all arguments and fields in *.pyi files are annotated. There was no signature check and missing or superfluous annotations.

I also tried pytype but it fails loading C modules. I've also tried mypy's stubgen but this generates only empty stubs. I'll try to debug either of it to get it working and automate generation of *.pyi information first.

@wiktorn
Copy link
Contributor Author

wiktorn commented Oct 26, 2018

I've just found python/mypy#5814. Looks promising and was merged just 2 days ago. I gave it a try on current (boost_python) version and results are better than before, but still far from perfect (and even acceptable), as all C++ function signatures are guessed as (*args, **kwargs). I hope that it will be better with pybind as this was primary goal of mentioned issue.

@lonvia
Copy link
Member

lonvia commented Oct 27, 2018

Thanks for looking into this. If you want to play with the pybind version, the current state is on branch https://github.com/osmcode/pyosmium/tree/pybind-rewrite. It only works with python3 at the moment and some parts are still missing but it should be enough to play with mypy. You need to put a copy of pybind11 in contrib/pybind11 yourself. I still need to figure out how to best handle this external dependency (submodule, copy, external install, ...).

@wiktorn
Copy link
Contributor Author

wiktorn commented Nov 20, 2018

Just to let you know. My current plan to move this forward is:

So this will achieve the goal of not having to think about pyi files when implementing changes.

Another approach to consider is to extend pybind11 with the ability to generate pyi files for classes it generates, but this way above my current C++ skills

@lonvia
Copy link
Member

lonvia commented Nov 25, 2018

Sounds all good to me. And I see that your mypy change got merged already. I couldn't help with pybind11 either, as my C++ skills do not include that level of template magic.

@wiktorn
Copy link
Contributor Author

wiktorn commented Feb 3, 2019

Using master branch of mypy/stubgenc it is possible now to get almost perfect stubs.

The only outstanding broken annotation are in lib/osmium.cc:

def apply(reader: osmium.io.Reader, handler: osmium::handler::NodeLocationsForWays<osmium::index::map::Map<unsignedlong, osmium: :Location>, osmium: :index::map::Dummy<unsignedlong, osmium: :Location>>) -> None: ...
def apply(reader: osmium.io.Reader, node_handler: osmium::handler::NodeLocationsForWays<osmium::index::map::Map<unsignedlong, osmium: :Location>, osmium: :index::map::Dummy<unsignedlong, osmium: :Location>>, handler: BaseHandler) -> None: ...

But I didn't find a way to provide proper types here (like it was possible elsewhere by adding py::import statements).

The other outstanding problem is that pybind generates docstring for stl iterators that doesn't give any type information, but that something I hope can by addressed on pybind side.

I've added step in setup.py / build_ext that generates stubs for C++ modules. For Python modules - typing information is included as comments to maintain compatibility with Python2.

I've added build-requirements.txt and test-requirements.txt so it is easy to setup build/test environment with pip install -r build-requirements.txt etc.

PS. We will have to wait for mypy > v0.660 to get functionality from: python/mypy#5975

@lonvia
Copy link
Member

lonvia commented Sep 13, 2022

Closing in favour of #211.

@lonvia lonvia closed this Sep 13, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

2 participants