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

Added a get method to Opts to get an Options object #3440

Merged
merged 3 commits into from Feb 5, 2019
Merged

Conversation

jlstevens
Copy link
Contributor

@jlstevens jlstevens commented Jan 29, 2019

Prompted by a stackoverflow question I decide to go ahead and implement the get method on Opts we have previously discussed. It doesn't do anything fancy for container types but does let you specify the option type if you like e.g with points.opts.get('style').

I haven't put too much thought into the name: get was the first thing that came to mind.

@philippjfr
Copy link
Member

Looks good, I'd also like to see an optional backend kwarg and proper Google style docstring.

@jlstevens
Copy link
Contributor Author

I'd also like to see an optional backend kwarg...

Sure.

... and proper Google style docstring.

I did consider it but saw all the other docstrings in that class weren't google style so I opted to be consistent. I guess I can update all the methods of that class to be google style - maybe the trick to update all the docstrings across HoloViews is to work class by class instead of file by file (or method by method).

@philippjfr
Copy link
Member

Looks good to me, I'll merge once tests pass.

@jlstevens
Copy link
Contributor Author

jlstevens commented Feb 4, 2019

I did consider it but saw all the other docstrings in that class weren't google style so I opted to be consistent.

Little confused as the other docstrings were google style...I must have been looking at the wrong class when I said that!

@philippjfr
Copy link
Member

PR builds passed, merging.

@philippjfr philippjfr merged commit cf2fc33 into master Feb 5, 2019
@philippjfr philippjfr deleted the opts_get_API branch April 29, 2019 11:25
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

2 participants