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

Allow a param.Selector with no objects to be turned into an AutocompleteInput widget #2966

Merged
merged 3 commits into from
Apr 4, 2022

Conversation

maximlt
Copy link
Member

@maximlt maximlt commented Nov 30, 2021

Minor fix to allow a Selector Parameter that is defined with no objects from the beginning to be turned into an AutocompleteInput widget.

@jbednar
Copy link
Member

jbednar commented Nov 30, 2021

Presumably an AutocompleteInput widget returns only strings, but a Selector with no items won't have any such restriction. Will this fix also apply to a ClassSelector with a class_ of str, which I think would be a more direct way to express this behavior (as it lets downstream code reason about the type of the value better)?

@codecov
Copy link

codecov bot commented Nov 30, 2021

Codecov Report

Merging #2966 (8d4d6c0) into master (afd555a) will increase coverage by 0.01%.
The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #2966      +/-   ##
==========================================
+ Coverage   83.46%   83.47%   +0.01%     
==========================================
  Files         190      190              
  Lines       25206    25217      +11     
==========================================
+ Hits        21038    21050      +12     
+ Misses       4168     4167       -1     
Impacted Files Coverage Δ
panel/param.py 87.20% <100.00%> (+0.16%) ⬆️
panel/tests/test_param.py 99.88% <100.00%> (+<0.01%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update afd555a...8d4d6c0. Read the comment docs.

@maximlt
Copy link
Member Author

maximlt commented Dec 8, 2021

It is true that a Selector doesn't put any constraint on the type of data it contains (I thought item_type was supported). But the intention really here is to allow a user to declare an empty Selector Parameter, whose objects get populated at some point, for instance after a database query. This is already possible with a Select widget, but not AutocompleteInput.

Isn't a ClassSelector Parameter with a single str class as an option pretty much the same thing as a String Parameter? Currently a String Parameter is mapped by Panel to a TextInput.

@jbednar
Copy link
Member

jbednar commented Dec 9, 2021

the intention really here is to allow a user to declare an empty Selector Parameter, whose objects get populated at some point, for instance after a database query. This is already possible with a Select widget, but not AutocompleteInput.

But won't that database query always return the same type of object? In that case it should be a ClassSelector, which is meant for a collection of items in the same class, not a Selector, which is meant for a specific set of concrete items, probably but not necessarily sharing the same class. Where it matters is for downstream code -- can it assume that the result is of a particular class or shares a certain API? If so, that should be declared rather than assumed.

Isn't a ClassSelector Parameter with a single str class as an option pretty much the same thing as a String Parameter?

At the Param level, a ClassSelector with class_=str, is_instance=True, and check_on_set=False is pretty much the same as a String Parameter. But at the Panel level, a String parameter has no notion of a list of allowed values, while a ClassSelector will keep a list of objects that a GUI can suggest either as options in a dropdown or as autocompletions. A user can override that list if check_on_set is False, but it does exist and the GUI can make use of that.

@maximlt
Copy link
Member Author

maximlt commented Dec 10, 2021

But won't that database query always return the same type of object?

Yes. But I'm not sure I follow your point, how can I declare a ClassSelector that holds a collection of items? I've seen more the case where ClassSelector is used to declare that a Parameter can be of different types, e.g. param.ClassSelector(class_=(list, dict)).

ClassSelector has no check_on_set argument.

@jbednar
Copy link
Member

jbednar commented Dec 10, 2021

Ah, good point. It looks like ClassSelector won't actually use its objects list, invalidating my point. And thus ClassSelector doesn't need a check_on_set argument, which would effectively always be False.

Ok, I'm back to the original issue that using an AutocompleteInput with Selector will always generate a string result, and there is I guess currently no way to declare that constraint in a way that downstream code can rely on. So we'll have the situation that people will instantiate a Selector, happily use it with AutocompleteInput, assume that the result will always be a string, and yet Param won't actually enforce that it is a string, causing problems for anyone who uses that code in a context not driven by a Panel AutocompleteInput. To prevent such problems, it looks like either the class_ slot would need to move up into Selector, optionally letting a Selector declare and enforce the type of what's allowed for selection, or else ClassSelector would need to do something with the objects list (which it doesn't explicitly use right now from what I can see, though maybe some such behavior is inherited).

@maximlt
Copy link
Member Author

maximlt commented Dec 11, 2021

Ok I understand better why I was confused by your suggestions now! ClassSelector is like Selector a subclass of SelectorBase, but SelectorBase is just an abstract method that declares a get_range method. So ClassSelector has no objects attribute.

It'd be indeed useful to have a Parameter that has a list of possible objects which are of one or more declared types.

@maximlt
Copy link
Member Author

maximlt commented Jan 12, 2022

I've merged master to resolve conflicts and fix the CI. What do you think about this PR @philippjfr ?

@philippjfr philippjfr merged commit 0291d21 into master Apr 4, 2022
@philippjfr philippjfr deleted the fix_autocomplete_param_no_objects branch April 4, 2022 13:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants