PR: Add compatibility mappings between bindings for all children of QSinglePointEvent#417
Conversation
|
Since this is a bugfix to an existing change in 2.3.1, it seems like it would be nice to get this in that release also before the previous change ships, to avoid any discruption.
Maybe can be omitted for now, or just check that they have the expected |
…letEvent`, and `QHoverEvent`
|
Added dummy checks ( As a side note, would
I'd like to try, but not in a PR here. |
QSinglePointEventQSinglePointEvent
CAM-Gerlach
left a comment
There was a problem hiding this comment.
One comment on how to simplify the tests
That actually is what the review process for. Thanks, @CAM-Gerlach! One of the tests failed, but it's not me this time. I wish there was a way to stabilize the test environment and reuse it to speed the tests up. |
|
I restarted the failed job. If it keeps happening, I can look into it further or explore other options like locked dependencies or additional caching. |
QSinglePointEventQSinglePointEvent
dalthviz
left a comment
There was a problem hiding this comment.
Thanks @StSav012 for the work here! This LGTM 👍 but just in case, what do you think @ccordoba12 ? And also, is there any other thing that should be addressed before merging this @CAM-Gerlach ?
There was a problem hiding this comment.
Thanks @StSav012! I left some small suggestions for you to avoid very long lines in the code, otherwise looks good to me.
CAM-Gerlach
left a comment
There was a problem hiding this comment.
LGTM, at least as far as my expertise is concerned 👍
|
Thanks, @ccordoba12! What line length do you prefer? I stick to 120 char limit. I'll gladly follow your code style to save you the extra work on cosmetics. @CAM-Gerlach, please apply such formatting changes without consulting me to save time (especially when a commit delays a release). Living in Europe, I read the comments many hours after they appear. |
Co-authored-by: Carlos Cordoba <ccordoba12@gmail.com>
At least in most Spyder projects, nominally 79 characters per PEP 8, but the current code is somewhat inconsistent until we blacken it (and even then, we might want to go with the default 88 characters to avoid too much churn and to avoid too many line breaks on the large number of complex lines QtPy has). I've tried to avoid too many style nitpicks myself except when in the course of another suggestion.
Sure, I can do that for you in the future if you like; we just generally like to be very respectful of the author's wishes and not do so without their explicit permission (which you've now given) to avoid any friction and misunderstandings (as has happened in the past). Thanks! |
Analyzing #408, I've noticed that the functions I made for Qt6 are not actually of
QMouseEvent, but belong to its base classQSinglePointEvent. So, that's the quick fix for that. I'm sorry I haven't done this several days earlier.Unfortunately, I couldn't think of good tests for
QNativeGestureEvent,QEnterEvent,QTabletEvent,QHoverEvent, and(edit: it has always had Qt6-styleQWheelEventpositionandglobalPosition), which all are the child classes ofQSinglePointEventin Qt6. I'd really appreciate some help here.