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 PositionX/Y/XY streams to work with categorical axes #1094

merged 1 commit into from Jan 31, 2017


Copy link

@philippjfr philippjfr commented Jan 31, 2017

Changes the parameter types of these stream types to allow strings from categorical axes, allowing them to work with HeatMap and other categorical plots.

@philippjfr philippjfr requested a review from jlstevens Jan 31, 2017
Copy link

@jbednar jbednar commented Jan 31, 2017

Looks good. Is numbers.Number better or worse than (int, float)?

Copy link

@jlstevens jlstevens commented Jan 31, 2017

Although the use of ClassSelector makes the definition of PositionXY look less trivial than before (which I liked), people using this stream class won't notice any difference relative to the old version (unless they look at the docs) and the new version will now work in more cases.

Therefore, I think this approach is better in terms of usability which outweighs the value of having a nice, trivial stream definition in the code. Merging.

@philippjfr philippjfr force-pushed the heatmap_position_stream branch from 93fe86e to 12ccdbd Jan 31, 2017
@jlstevens jlstevens merged commit 7041e28 into master Jan 31, 2017
0 of 2 checks passed
0 of 2 checks passed
continuous-integration/travis-ci/pr The Travis CI build is in progress
continuous-integration/travis-ci/push The Travis CI build is in progress
@philippjfr philippjfr deleted the heatmap_position_stream branch Feb 10, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
None yet
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

3 participants