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

Fixed scaling issue with :meth:.BarChart.change_bar_values #1826

Merged
merged 6 commits into from Jul 29, 2021

Conversation

noamzaks
Copy link
Contributor

@noamzaks noamzaks commented Jul 23, 2021

Changelog / Overview

This closes #1820 by scaling to the height of the bar area instead of the whole VGroup height (which is larger because of the Y-axis at the bottom). It also sets the width and height of the bar area as members so they can be used in future methods.

Fix bug with change_bar_values in :class:~.BarChart

Motivation

The bar chart scales correctly and future methods can use the area of the chart itself.

Testing Status

All tests except test_graphical_units/test_opengl passed on macOS 11

Checklist

  • I have read the Contributing Guidelines
  • I have written a descriptive PR title (see top of PR template for examples)
  • I have written a changelog entry for the PR or deem it unnecessary
  • My new functions/classes either have a docstring or are private
  • My new functions/classes have tests added and (optional) examples in the docs
  • My new documentation builds, looks correctly formatted, and adds no additional build warnings

Reviewer Checklist

  • The PR title is descriptive enough
  • The PR is labeled correctly
  • The changelog entry is completed if necessary
  • Newly added functions/classes either have a docstring or are private
  • Newly added functions/classes have tests added and (optional) examples in the docs
  • Newly added documentation builds, looks correctly formatted, and adds no additional build warnings

Copy link
Member

@hydrobeam hydrobeam left a comment

Choose a reason for hiding this comment

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

This looks good to me, thanks for the contribution!

The current way that bar charts are created is quite convuluted, so when issue #1703 is handled it should hopefully prevent bugs like this 👍

@hydrobeam hydrobeam added the pr:bugfix Bug fix for use in PRs solving a specific issue:bug label Jul 27, 2021
@behackl behackl enabled auto-merge (squash) July 29, 2021 16:22
@behackl behackl merged commit 018c763 into ManimCommunity:main Jul 29, 2021
@WampyCakes WampyCakes changed the title Fix bar chart scaling Fixed :class:~.BarChart scaling Aug 1, 2021
@behackl behackl changed the title Fixed :class:~.BarChart scaling Fixed scaling issue with :meth:.BarChart.change_bar_values Aug 1, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
pr:bugfix Bug fix for use in PRs solving a specific issue:bug
Projects
None yet
Development

Successfully merging this pull request may close these issues.

The bars are higher than the values provided to the change_bar_values method of BarChart class
3 participants