Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Restrict JuliaItemModel constructor to match documentation and behavior #205

Open
steffenhaug opened this issue Aug 21, 2024 · 3 comments
Open

Comments

@steffenhaug
Copy link

The manual states that JuliaItemModel is constructed from a 1D Julia array, but the method currently silently accepts types that are not arrays, without producing a MethodError immediately (as i believe it should since the argument actually needs to be an array) only to then fail down the line. For example:

julia> JuliaItemModel(nothing)
QML.JuliaItemModelAllocated(Ptr{Nothing} @0x0000600003a95660)

I'm wondering if JuliaItemModel method should be restricted to work only with types which are AbstractArray?

Perhaps more insidiously, things that in principle are natural, such as

JuliaItemModel(Observable([1,2,3]))

are accepted, but fails down the line, without making the root cause clear.

@steffenhaug
Copy link
Author

As a sidenote, I did some type piracy to support the Observable use case:

function QML.JuliaItemModel(obs::Observable{T}) where {T<:AbstractArray}
  model = JuliaItemModel(obs[])

  on(obs) do data
    values(model)[] = data
    QML.force_model_update(model)
  end

  return model
end

I don't know if this implementation has pitfalls; maybe it can be improved. I could work on a PR if you think this use case is worth supporting.

@barche
Copy link
Collaborator

barche commented Aug 23, 2024

Thanks for the comments. I think we should actually update the documentation, since it is not strictly needed for the data to be an array, for example it can be a DataFrame. The idea is that the user can provide appropriate overrides for methods that operate on ItemModelData, e.g. QML.insert_row!(m::QML.ItemModelData{DataFrame}, rowidx, row::AbstractVector{QVariant}).

I'm not sure about pitfalls in passing an observable, but note that values(model) is actually already an observable, with the intention to be able to act on changes coming from QML.

@steffenhaug
Copy link
Author

Thanks for the clarification.

I am aware that values(model) is an observable, and that changes from QML can be acted on from Julia, but the QML side can not currently act on the observable getting assigned from the Julia side. As far as I understand, it will only be notified if you update the model using QMLs interface.

That is, values(model)[] = new_data will not propagate to the QML side without a forced model update, so my idea was simply to equip models containing Observable data with a signal handler that propagates the notification to QML. I suppose you could also attach a signal handler to values(model) that performs a model update. That way all JuliaItemModels will propagate reassignments to QML.

For some context, my use case involves repeatedly populating a JuliaItemModel with data from a database based on some parameters the users choose, so the data can be quite unpredictable, and it is not practical to update the model by inserting and deleting elements.

Anyways, bridging observable notifys to QML is probably a separate issue, I can open one so it is more easy to search for.

I think this issue, the way I phrased it, is adequately solved by clarifying the documentation. As far as I can tell, the methods operating on ItemModelData are not documented in the manual, but with your clarification and poking around the source code for a bit, it seems like I could implement what I want by overriding clear! and append_row!, although I'm a bit worried this would emit O(n) state updates to QML. Maybe there should be a method to overload to do a complete reassignment of the data? (Maybe there is and I haven't found it!)

And to end on a positive note; I have to say that QML on the whole has been a joy to use. Bridging Observables and Makie to Qt is magical :-)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants