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

BarChart[] #499

Closed
wants to merge 11 commits into from
Closed

BarChart[] #499

wants to merge 11 commits into from

Conversation

poke1024
Copy link
Contributor

chart-example

still needs some error checks for a basic version.

still lacks legends.

y = x.get_float_value()
if y is None:
return 0.
return y
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this would be better implemented with round_to_float(evaluation=evaluation). This one doesn't handle non-machine reals and symbolic arguments.

@sn6uv
Copy link
Member

sn6uv commented Aug 29, 2016

Also, is palettable strictly necessary? I think BarChart is 'standard enough' that we should try to make it work by default (without having to install additional libs). Another option would be just to install it by default but maybe we could replicate the functionality easily enough?

@poke1024
Copy link
Contributor Author

We could add our own color tables, but it's not that easy to get good palettes and copying these or the palettable palettes always poses license issues. palettable is quite small (the color palette values plus helper methods plus some docs and tests, see https://github.com/jiffyclub/palettable) and it doesn't bring in any additional dependencies (the only completely optional dependency is matplotlib, but it only interfaces if you install matplotlib manually). It is documented to work Python 2 and Python 3 and I used it under PyPy without any problems. It solves the license brouhaha we'd have to solve if we copied their (or other) palettes into our code. The other option would obviously be our own original palettes, but I'm not sure where got get those or how to produce them. So I'm quite in favour of making palettable a default install, as it gives us very nice palettes without any headaches.

@sn6uv
Copy link
Member

sn6uv commented Aug 31, 2016

Okay. Good points. Let's add it as a default then.

@GarkGarcia
Copy link
Contributor

See #855 for updates.

@GarkGarcia GarkGarcia closed this Sep 11, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement needs rabase An old PR that needs to be rebased
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants