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

StyleAlias proposal for use in __call__ #512

Closed
jlstevens opened this issue Feb 17, 2016 · 14 comments
Closed

StyleAlias proposal for use in __call__ #512

jlstevens opened this issue Feb 17, 2016 · 14 comments
Assignees
Labels
tag: API type: feature A major new feature

Comments

@jlstevens
Copy link
Contributor

We've been discussing the use of __call__ to set styles in #508 and here I propose something related, designed to make styling easier.

Here is a relatively simple example of __call__ from a notebook Philipp was working on:

StockImage(group='StockImage')(plot=dict(projection=crs.Mollweide()))

We agree that this styling syntax is clumsy but I don't agree it should be changed. Instead, I think we should offer a useful utility just like the alias utility described in #312:

style = StyleAlias(earth="Image [cmap='Greens']", mars="Image [cmap='Reds']")

This utility helpfully lets you define multiple styles in one place and it would parse the opts magic syntax when defined (sadly we can't help tab complete in this case). If you don't have IPython installed (or pyparsing installed) you could just supply the usual dictionary syntax instead of a string. Then you could use:

StockImage(group='StockImage')(style.earth)  #Or style['earth']

This access would expand to the standard formats we already accept in __call__. This means this utility is entirely optional and is there simply for making styling via __call__ easier.

@jbednar
Copy link
Member

jbednar commented Feb 17, 2016

As Jean-Luc and I discussed just now, I don't think we should be providing special objects for the user that are essentially just fancy dictionaries, in any domain-specific bits of the code like this one. Instead, if we want our users to be able to have fancy dictionaries, we should simply provide such an object separately and generally, i.e. a full-featured fancy dictionary that could be used anywhere for anything (like AttrDict used to be), and then only show in examples how to use them (or regular dictionaries) for convenient access like this, without ever relying on it. That way if a user wanted to have the convenient attribute access, they'd use AttrDict (or some better variant of it), and if they didn't, they'd use a regular dict, but HoloViews wouldn't have to know or care about it either way. They'd simply use it themselves, and supply HoloViews the result of the lookup, with no domain-specific code needed, and no change to HoloViews either way:

style = AttrDict(earth="Image [cmap='Greens']", mars="Image [cmap='Reds']") StockImage(group='StockImage')(style.earth)

style = dict(earth="Image [cmap='Greens']", mars="Image [cmap='Reds']") StockImage(group='StockImage')(style["earth"])

(where both work the same, as far as HoloViews is concerned; they simply store any valid option specifier, either the string ones here (which would require extensions to the __call__ syntax) or the usual dict of dicts ones).

Otherwise we're going to get a proliferation of objects that are all just specialized variants of a fancy dictionary. I think the same applies to the aliases object -- let's only have one, general fancy dictionary, not lots of special-purpose ones to document, remember, understand, and maintain!

@jlstevens
Copy link
Contributor Author

Ok, I'll summarize what I was writing as I see @jbednar has just posted basically the same thing. :-)

The plan is:

  • Let __call__ accept the string syntax directly (will needs pyparsing though).
  • Let the user manage their collections of styles (e.g with a normal Python dictionary).
  • Perhaps bring AttrDict as a convenience on the user side only. We should never make use of it in HoloViews itself. In other words, we offer the class definition in core.util and it is not mentioned again. We must not assume attribute access works for dictionary types anywhere in the code.

There is one point where I disagree though - the aliases object isn't a simple lookup as tuples are required to define group/label/name. That object already exists and works well so I don't see any good reason to replace it with AttrDict (and I don't think that would be possible anyway).

@jbednar
Copy link
Member

jbednar commented Feb 17, 2016

That all sounds right. For aliases, I haven't studied how it works, only how StyleAliases would work and the fact that you said that StyleAliases would work "just like" the label aliases. :-) Sure, if aliases still needs to be special, then fine, though I don't currently see what would be special about it.

@jlstevens jlstevens self-assigned this Feb 17, 2016
@philippjfr philippjfr added type: feature A major new feature tag: API labels Feb 25, 2016
@jlstevens
Copy link
Contributor Author

@jbednar @philippjfr I had forgotten about this suggestion and maybe this links into our recent discussion about the appropriate syntax for __call__ (now opts)?

Right now, opts supports the string syntax directly i.e:

image.opts("Image [cmap='Greens']", mars="Image [cmap='Reds']")

Is there a benefit to decoupling the parsing so you parse the string to get Python literals you can use?

@philippjfr
Copy link
Member

philippjfr commented Jun 20, 2017

Is there a benefit to decoupling the parsing so you parse the string to get Python literals you can use?

Yes a function that parses and then turns it into the dictionary format would make sense to me.

image.opts("Image [cmap='Greens']", mars="Image [cmap='Reds']")

As an aside what is mars here, I don't understand this syntax. (Looks like you copied the example above from the StyleAlias proposal, don't think it makes sense here).

@jlstevens
Copy link
Contributor Author

jlstevens commented Jun 20, 2017

Well spotted! I copied and pasted without thinking. As you predicted, I meant to use the usual opts string syntax.

I am starting to think there is room for a hv.style utility (or similar) that could offer a few things:

  • Parsing from string syntax to Python literals. Maybe it is best to decouple the parsing from opts and __call__ to make the literal syntax easier to work with without making it look like HoloViews works with string specifications (it doesn't really).
  • Tool supporting optional conversion to and from yaml/json (yaml would only be available if the required conditional dependency was met).
  • Other useful things maybe? E.g merging specifications? Pretty printing them (e.g sorting into alphabetical order by elements)?

I didn't invest any effort into making the string syntax work with opts (in fact there is an issue about improving this). The main thing was pulling the parser out of holoviews.ipython which makes it a core dependency. We mustn't forget to add it to meta.yaml and setup.py!

@philippjfr
Copy link
Member

No objections from me on your proposal except that hv.style is definitely not the right name for it.

@jlstevens
Copy link
Contributor Author

This is linked to issue #1578.

@philippjfr
Copy link
Member

I think this suggestion is obsolete now that we have hv.opts and Dimensioned.options, so please close unless you think there is another valuable suggestion in here (I really hope not because I don't think I can cope with another mechanism for setting options).

@jlstevens
Copy link
Contributor Author

I think this suggestion is orthogonal to .options: even though it is now easier to specify options, it might still be nice to define sets of them with an alias system.

I agree adding .options lower the priority of this suggestion and people can now probably just do options(**kwargs), though it might be nice to offer something a bit more structured than letting people define dictionaries as variables all of the place. In particular it might be nice to group things together and offer attribute/tab-completion access.

@philippjfr
Copy link
Member

Not particularly convinced by the utility of another specialized object people would have to know about but I'm happy to leave this open if you still think it's a good idea.

@jlstevens
Copy link
Contributor Author

I think there are helper utilities we could offer to help people structure their holoviews code better. That said, it is tricky to offer a utility that is very general, very helpful and also very simple to understand. I also don't want lots of tiny utilities to have to learn...

@philippjfr
Copy link
Member

I'm hoping we can close this now that we have: #3095

I really don't think we should introduce any more concepts in addition to what's outlined there.

Copy link

This issue has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Oct 24, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
tag: API type: feature A major new feature
Projects
None yet
Development

No branches or pull requests

3 participants