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

Parser fixes #948

Merged
merged 4 commits into from
Oct 27, 2016
Merged

Parser fixes #948

merged 4 commits into from
Oct 27, 2016

Conversation

philippjfr
Copy link
Member

@philippjfr philippjfr commented Oct 24, 2016

Two small fixes for the parser, the first fixes passing dictionary literals by eliminating duplicate commas generated by the parser, fixing part of the issue described in #270, e.g. this is now allowed:

%%opts Scatter [fontsize={'title': '20pt'}]
hv.Scatter(np.random.rand(100,2), group='A')

Secondly it fixes a bug when a opts definition includes nested bracketing as described in #930, which described an issue where:

%%opts Curve (color=hv.Cycle(values=sns.color_palette('Set1')))
hv.Overlay([hv.Curve(np.arange(10)*i) for i in range(3)])

Was not evaluated correctly. This is now also correctly processed.

Note that this does not fix all issues mentioned in #270, e.g. aspect=dict(x=1, x=2, z=3) still fails to evaluate, but it provides a significant improvement in some cases.

@jlstevens
Copy link
Contributor

Parser improvements, yay!

All I will ask is that you add a bunch of unit tests. There are already a bunch here but you cannot have too many parser tests.

@philippjfr
Copy link
Member Author

Agreed, happy to add tests.

@philippjfr
Copy link
Member Author

Added some tests now, which ended up helping me figure out further issues. Still not perfect but it actually let's you do a fair number of more complex expressions now.

@jlstevens
Copy link
Contributor

Great to have more fixes and more tests! Merging.

@jlstevens jlstevens merged commit b64b6c5 into master Oct 27, 2016
@philippjfr philippjfr deleted the parser_fixes branch January 7, 2017 15:01
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