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

Adding three small features #65

Open
wants to merge 10 commits into
base: main
Choose a base branch
from
Open

Adding three small features #65

wants to merge 10 commits into from

Conversation

moravveji
Copy link

Hi.

I would like to propose three small modifications, in order to address the issue #60.

The major change is that now, the normalize() function basically uses the sum of each row of values in data, instead of the max_dat. This protects the case where max_dat < width against bad bar plots.

I also changed int() to round() to make sure that the bars (almost) add up to 100 when the data is expressed in percentage. Otherwise, the bars would not line up nicely.

Please let me know what you think of the changes.

Kind regards
Ehsan

@mkaz
Copy link
Owner

mkaz commented Jun 28, 2020

Thanks for the contribution!

I updated recently in 0.4.2 to fix the issue with max < width, do you think this change is still necessary?

The int() to round() seems like a good change.

It is difficult to see your change with all of the linting changes, can you setup black formatter and run that on your change first, it should auto format and match the style of mine.

Installation: python -m pip install black
Run: black termgraph/termgraphy.py
Or run as a module if the above doesn't work: python -m black termgraph/termgraphy.py

You can also setup your Editor to auto format, instructions here, depends on what editor you use.

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.

2 participants