mbdevpl / horast

Human-oriented abstract syntax tree (AST) parser/unparser for Python 3 that doesn't discard comments.
Apache License 2.0
16 stars 6 forks source link

Make it PEP 561 compatible #1

Closed ZdenekM closed 5 years ago

ZdenekM commented 5 years ago

Would you mind to make it PEP 561 compatible (see https://mypy.readthedocs.io/en/latest/installed_packages.html)? It basically means to create file py.typed in the package and add it to package_data - I would do it myself and create PR, but it somehow does not work with your setup_boilerplate - probably some trivial problem :)

Btw, thanks for your great work - horast was exactly what I needed...

mbdevpl commented 5 years ago

Thanks for trying out horast!

Yes, actually recently I discovered a bug in my boilerplate, which causes errors in all of my packages. I'm gradually propagating the fix to all packages, I think I just didn't get to horast yet. Basically now it requires to install dependencies manually via pip3 install -r test_requirements.txt if you use some other way to install than most common pip3 install horast. But it might be not related at all to your issue. What error(s) are you seeing?

And also, when I add a new non-python file to a project, I simply add it to MANIFEST.in. I'm doing that since long time ago, and actually I've been wondering is it a "good" approach or not. I think also for py.typed it would be enough. But if it's not the correct approach anymore, please tell me, I'd rather use the correct approach then.

mbdevpl commented 5 years ago

Oh well, actually that boilerplate bug is fixed already in horast. And I added py.typed to the manifest, it seems to get packaged correctly. I'll merge after all the tests pass.

Still, I'd like to know about the errors you get from the boilerplate, as well as your take on manifest vs package_data vs some other approach, if you can spare time to write about these.

ZdenekM commented 5 years ago

Thanks a lot, I can confirm that mypy can now find the package. Actually, I'm pretty sure that error was on my side - I didn't study boilerplate code, just put package_data into Package or something like that. While developing in Python for long time, I'm completely new to packaging (previously, I used mainly ROS which has its own way how to deal with packages) so I cannot asses if the approach with MANIFEST.in is good or not :)

mbdevpl commented 5 years ago

Actually just putting package_data into Package should work - that's how I designed the boilerplate. But fair enough, if you're not 100% sure what you did then never mind :) Thanks for confirming that it works with mypy now. Ok then, I'll release the new version as-is.