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

Add initial Pie Chart implementation #591

Merged

Conversation

JohnBarton27
Copy link
Contributor

I wanted to add a Pie Chart to a Sheet I was populating using pygsheets. I added an initial implementation here that extends the Chart class with a few key changes. Definitely a little bit of duplicated/similar code in the methods that get overridden in the PieChart class - open to suggestions if you want a deeper re-write to abstract some of that out. Hopefully this can also help build a pattern for other future "non-basic" Charts to get support.

Copy link
Owner

@nithinmurali nithinmurali left a comment

Choose a reason for hiding this comment

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

Overall looks good. just some minor comments.

@@ -1676,6 +1677,19 @@ def add_chart(self, domain, ranges, title=None, chart_type=ChartType.COLUMN, anc
"""
return Chart(self, domain, ranges, chart_type, title, anchor_cell)

def add_pie_chart(self, domain, chart_range, title=None, anchor_cell=None, three_dimensional=False, pie_hole=0):
"""
Similar to `add_cart`, but created a Pie Chart instead of a Basic Chart.
Copy link
Owner

Choose a reason for hiding this comment

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

typo: add_cart should be add_chart

tests/online_test.py Outdated Show resolved Hide resolved
@JohnBarton27
Copy link
Contributor Author

Overall looks good. just some minor comments.

Sounds good, just pushed fixes!

@nithinmurali
Copy link
Owner

Awesome! thanks for the PR.

@nithinmurali nithinmurali merged commit 033334e into nithinmurali:staging Jan 14, 2024
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