{"action":"created","issue":{"url":"https://api.github.com/repos/pybee/voc/issues/122","repository_url":"https://api.github.com/repos/pybee/voc","labels_url":"https://api.github.com/repos/pybee/voc/issues/122/labels{/name}","comments_url":"https://api.github.com/repos/pybee/voc/issues/122/comments","events_url":"https://api.github.com/repos/pybee/voc/issues/122/events","html_url":"https://github.com/pybee/voc/pull/122","id":148641214,"number":122,"title":"Rich/direct comparison methods","user":{"login":"cflee","id":8225175,"avatar_url":"https://avatars.githubusercontent.com/u/8225175?v=3","gravatar_id":"","url":"https://api.github.com/users/cflee","html_url":"https://github.com/cflee","followers_url":"https://api.github.com/users/cflee/followers","following_url":"https://api.github.com/users/cflee/following{/other_user}","gists_url":"https://api.github.com/users/cflee/gists{/gist_id}","starred_url":"https://api.github.com/users/cflee/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/cflee/subscriptions","organizations_url":"https://api.github.com/users/cflee/orgs","repos_url":"https://api.github.com/users/cflee/repos","events_url":"https://api.github.com/users/cflee/events{/privacy}","received_events_url":"https://api.github.com/users/cflee/received_events","type":"User","site_admin":false},"labels":[],"state":"open","locked":false,"assignee":null,"milestone":null,"comments":5,"created_at":"2016-04-15T11:52:45Z","updated_at":"2016-04-15T15:36:34Z","closed_at":null,"pull_request":{"url":"https://api.github.com/repos/pybee/voc/pulls/122","html_url":"https://github.com/pybee/voc/pull/122","diff_url":"https://github.com/pybee/voc/pull/122.diff","patch_url":"https://github.com/pybee/voc/pull/122.patch"},"body":"Ref #38:\r\n\r\n> > The “rich comparison” methods. When called directly (`{}.__lt__(1)`) they return `\"NotImplemented\"`, and when called using `{} < 1` they raise the following excpetion: `TypeError: unorderable types: dict() < int()`. What should the code do, then?\r\n\r\n> Implement `__lt__()` as if it were being invoked directly as a function. The code to convert the `NotImplemented` into a `TypeError` will go in the implementation of the `COMPARE_OP` Python opcode.\r\n\r\nThis PR:\r\n\r\n* Modifies various comparison methods like `__lt__()` to act as if they were called directly, such that they return `NotImplemented` instead of throwing `TypeError`, and also add the `args` value to the `@Method` annotations so that they can be invoked directly\r\n* Adds a new bunch of tests, `test_direct_lt_[datatype]` etc. to verify this\r\n* Adds a new helper `comparison` method to `org.python.Object`, which implements the [reflected operation logic](https://docs.python.org/3/reference/datamodel.html#object.__lt__)\r\n* Modifies `COMPARE_OP` opcode to invoke the helper `comparison` method instead of the comparator method directly\r\n\r\nIt should also make it easier to implement some of these comparison methods, as fewer combinations need to be handled on some of the basic types.\r\n"},"comment":{"url":"https://api.github.com/repos/pybee/voc/issues/comments/210509792","html_url":"https://github.com/pybee/voc/pull/122#issuecomment-210509792","issue_url":"https://api.github.com/repos/pybee/voc/issues/122","id":210509792,"user":{"login":"cflee","id":8225175,"avatar_url":"https://avatars.githubusercontent.com/u/8225175?v=3","gravatar_id":"","url":"https://api.github.com/users/cflee","html_url":"https://github.com/cflee","followers_url":"https://api.github.com/users/cflee/followers","following_url":"https://api.github.com/users/cflee/following{/other_user}","gists_url":"https://api.github.com/users/cflee/gists{/gist_id}","starred_url":"https://api.github.com/users/cflee/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/cflee/subscriptions","organizations_url":"https://api.github.com/users/cflee/orgs","repos_url":"https://api.github.com/users/cflee/repos","events_url":"https://api.github.com/users/cflee/events{/privacy}","received_events_url":"https://api.github.com/users/cflee/received_events","type":"User","site_admin":false},"created_at":"2016-04-15T15:36:34Z","updated_at":"2016-04-15T15:36:34Z","body":"> In a comment, I noted that `comparision` should be `__cmp__`; however, as you note in your first question, it's probably a moot point anyway, because that whole method should be in the opcode layer, not the Java code.\r\n\r\nI initially tried to do this directly in the opcode layer by compiling it as a separate function and transcribing the `javap` output into `opcodes.py`, but I was running into this `VerifyError`. (It was an `astore_2` related thing earlier but essentially it's the `Local variable table overflow` that is the issue?)\r\n\r\n```\r\nError: A JNI error has occurred, please check your installation and try again\r\nException in thread \"main\" java.lang.VerifyError: Local variable table overflow\r\nException Details:\r\n  Location:\r\n    python/test/__init__.<clinit>()V @233: aload_1\r\n  Reason:\r\n    Local index 1 is invalid\r\n  Bytecode:\r\n    0x0000000: 014b b200 0813 000e bb00 0359 1300 10b8\r\n    0x0000010: 0012 b700 18b9 001c 0300 57bb 0022 59bb\r\n    0x0000020: 0024 5905 b700 2659 bb00 2959 04b7 002b\r\n    0x0000030: b900 2c02 0057 59bb 0029 5905 b700 2bb9\r\n    0x0000040: 002c 0200 57b7 0032 4bb2 0008 1300 0eb9\r\n    0x0000050: 0035 0200 c000 0313 0039 2ab9 003b 0300\r\n    0x0000060: bb00 2259 bb00 2459 04b7 0026 59bb 0029\r\n    0x0000070: 5906 b700 2bb9 002c 0200 57b7 0032 4bb2\r\n    0x0000080: 0008 1300 0eb9 0035 0200 c000 0313 0041\r\n    0x0000090: 2ab9 003b 0300 b200 0813 000e b900 3502\r\n    0x00000a0: 00c0 0003 1300 43b9 0045 0200 c000 4904\r\n    0x00000b0: bd00 3c59 03b2 0008 1300 0eb9 0035 0200\r\n    0x00000c0: c000 0313 0039 b900 4502 00b2 0008 1300\r\n    0x00000d0: 0eb9 0035 0200 c000 0313 0041 b900 4502\r\n    0x00000e0: 00b2 004b 4b2a b600 512b b600 51a5 001f\r\n    0x00000f0: 2ab6 0051 2bb6 0057 a500 142b 2ab9 005b\r\n    0x0000100: 0200 4b2a b200 4ba5 0005 2db0 2a2b b900\r\n    0x0000110: 5b02 004b 2ab2 004b a500 052a b01c a000\r\n    0x0000120: 142b 2ab9 005b 0200 4b2a b200 4ba5 0005\r\n    0x0000130: 2ab0 bb00 5f59 1300 6106 bd00 5259 032a\r\n    0x0000140: b900 6301 0053 5904 1300 6753 5905 2bb9\r\n    0x0000150: 0063 0100 53b8 0068 b700 6ebf 53bb 0071\r\n    0x0000160: 59b7 0073 b900 7503 0057 b200 7957 b1b1\r\n    0x0000170:                                        \r\n\r\n\tat java.lang.Class.getDeclaredMethods0(Native Method)\r\n\tat java.lang.Class.privateGetDeclaredMethods(Class.java:2701)\r\n\tat java.lang.Class.privateGetMethodRecursive(Class.java:3048)\r\n\tat java.lang.Class.getMethod0(Class.java:3018)\r\n\tat java.lang.Class.getMethod(Class.java:1784)\r\n\tat sun.launcher.LauncherHelper.validateMainClass(LauncherHelper.java:544)\r\n\tat sun.launcher.LauncherHelper.checkAndLoadMain(LauncherHelper.java:526)\r\n```\r\n\r\nBasically if there's some way to get space to store and retrieve variables (object references and booleans) it would be very helpful, otherwise I'll probably try to figure out how to get the references to the right spot on operand stack by hand using `DUP` and stuff. It seems that the x and y are already placed on the operand stack just before this `COMPARE_OP`'s bytecode starts, since that's where the `invokeinterface` for `org.python.Object.[comparator]` would happen.\r\n\r\n> I'm also wondering if the reflected operations docs you referenced has a bug in it - shouldn't `__ge__` be the reflection of `__lt__`, not `__le__`? Similarly for `__le__` and `__gt__`, instead of __ge__?\r\n\r\nIt is a little strange but unfortunately it doesn't seem to be a bug. Try this:\r\n\r\n```python\r\nclass Ge:\r\n    def __init__(self, val):\r\n        self.val = val\r\n    def __ge__(self, other):\r\n        return self.val\r\n\r\nx = 5\r\ny1 = Ge(True)\r\n\r\n# True: using __ge__\r\nprint(y1 >= x)\r\n# True: __le__ reflects to __ge__\r\nprint(x <= y1)\r\n\r\n# TypeError: __lt__ reflects to __gt__ (not implemented) not __ge__\r\ntry:\r\n    print(x < y1)\r\nexcept TypeError as err:\r\n    print(err)\r\n```"}}