adriank / ObjectPath

The agile query language for semi-structured data
http://objectpath.org
MIT License
380 stars 93 forks source link

Fix is/is not operators applied to None result #77

Closed yohanboniface closed 5 years ago

yohanboniface commented 5 years ago

This should fix #73

Side note: a run of flake8/black on the files should help new contributors :)

adriank commented 5 years ago

I added the yapf support.

adriank commented 5 years ago

Thanks for the PR, but I had to do it a little bit differently.

yohanboniface commented 5 years ago

Thanks for the PR, but I had to do it a little bit differently.

OK, thanks!

I added the yapf support.

Seems to me there are still mixed spaces and tab, and other pep 8 infrigements :( Honestly, if you are not confident with pep8, just use black, it will make your life easier, and the files much more welcoming for new contributors. :)

Also, are you planning to push a new release on pypi? :)

adriank commented 5 years ago

PEP8 is very opinionated and the community wasn't so religious about it in 2010 when I wrote ObjectPath. The library is not trivial and PEP8 is just not enough to make the code readable.

I uploaded ObjectPath 0.6 to PyPi just an hour ago.

Greetings, Adrian Kalbarczyk

http://kalbarczyk.co

On Mon, Nov 26, 2018 at 10:13 PM Yohan Boniface notifications@github.com wrote:

Thanks for the PR, but I had to do it a little bit differently.

OK, thanks!

I added the yapf support.

Seems to me there are still mixed spaces and tab, and other pep 8 infrigements :( Honestly, if you are not confident with pep8, just use black https://pypi.org/project/black/, it will make your life easier, and the files much more welcoming for new contributors. :)

Also, are you planning to push a new release on pypi? :)

— You are receiving this because you modified the open/close state. Reply to this email directly, view it on GitHub https://github.com/adriank/ObjectPath/pull/77#issuecomment-441799877, or mute the thread https://github.com/notifications/unsubscribe-auth/AAKycq1y1sqwu0z0VkR1_Gv8hzqOT1ITks5uzFlxgaJpZM4Ywn7Y .

yohanboniface commented 5 years ago

PEP8 is just not enough to make the code readable.

Still a good path forward :)

I uploaded ObjectPath 0.6 to PyPi just an hour ago.

Thanks! :)

adriank commented 5 years ago

symbol_table = {} token = nextToken = None

TODO optimization ('-',1) -> -1

TODO optimization operators should be numbers

TRUE = ["true", "t"] FALSE = ["false", "f"] NONE = ["none", "null", "n", "nil"]

How would you make this code compatible with C0103? If you'd use upper case for the symbol_table and token you would misinform other coders. They'd expect these variables to be constants while one is a container for variables and the other two are variables.

C0123 R0911 R0912 made code slow back in 2010 and I believe this is the case also right now.

W0703 makes writing ObjectPath in Python impossible.

etc.

Greetings, Adrian Kalbarczyk

http://kalbarczyk.co

On Mon, Nov 26, 2018 at 10:49 PM Yohan Boniface notifications@github.com wrote:

PEP8 is just not enough to make the code readable.

Still a good path forward :)

I uploaded ObjectPath 0.6 to PyPi just an hour ago.

Thanks! :)

— You are receiving this because you modified the open/close state. Reply to this email directly, view it on GitHub https://github.com/adriank/ObjectPath/pull/77#issuecomment-441811998, or mute the thread https://github.com/notifications/unsubscribe-auth/AAKycoTSLrRljGGr8G1NJVS5jdByesyhks5uzGH4gaJpZM4Ywn7Y .