MakieOrg / Makie.jl

Interactive data visualizations and plotting in Julia
https://docs.makie.org/stable
MIT License
2.4k stars 307 forks source link

Use `mouseposition_px` instead of `.mouseposition[]` in Events docs #2825

Open KronosTheLate opened 1 year ago

KronosTheLate commented 1 year ago

The section of the docs I am talking about it this one.

The example uses mp = events(scene).mouseposition[]. In trying to appropriate this, I in fact needed mouseposition(current_axis()). It took a long time to realize that a convenient function existed that did exactly what I needed, because it looked like the proper way to do it was to access the fields of events(scene), and I spent some time diving into the fields of Scene, Figure, Axis, just looking for the mouse position in data coordinates. This was fruitless for me.

If the examples used mp = mouseposition_px(current_axis()) (or the equivalent without using current_axis(), I am not sure about best practices), I think it would make mouseposition more discoverable. It is also part of the Julia style guide to use functions and not access internal fields directly, as it leaves more room for changing internals without having to change the API.

I am clearly biased, that my solution is what I needed. Do others also see value in changing the example to mp = mouseposition_px(current_axis())?

ffreyer commented 1 year ago

You need to be a little bit careful with mouseposition_px because it's relative to the particular scene it's called on. If the scene starts at (x, y), mouseposition_px will consider that point as (0, 0). The Events struct is shared between all scenes so it doesn't/can't do anything like that. Regardless of what you call events() on the events.mouseposition you read out will always be relative to the full window. For the moueposition() function there is the restriction that it only works in 2D. In 3D you should get a ray since you don't have enough information to get a point, but I think mouseposition will just return some point here.

I think this stuff should definitely be there. Perhaps another section that exemplifies the different situations and uses would be good. Might also be good to mention screen_relative and to_world in that context, which do the transformations in mouseposition_px() and mouseposition() (not sure if these are exported).

bjarthur commented 1 year ago

the OP can correct me if i'm wrong, but i believe what they're trying to say is that it's confusing that events(fig).mouseposition[] returns pixels and mouseposition(ax) returns data coordinates. i only found the latter because of https://github.com/MakieOrg/Makie.jl/issues/67#issuecomment-463180481. were the former renamed to mouseposition_px[], then users might think to look for the latter through docstrings instead of digging through issues.

KronosTheLate commented 1 year ago

My point was that if there is more functionality in the functions (ability to get data-coordinates and pixels), then it seems more flexible and less hacky to use accessor functions than accessing fields directly. But the point you make is also a good one for inconsistency. I guess the UI could be polished if the users are directed to a consistent set of functions, as opposed to accessing inconsistently names/inconsistently existing fields.