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

Fix issue where sectors showed a straight line instead of a curved #266

Merged
merged 2 commits into from Apr 22, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
7 changes: 7 additions & 0 deletions .github/workflows/tests.yml
Expand Up @@ -38,3 +38,10 @@ jobs:
run: |
pytest -s -rxs -vv -Werror tests/ --mpl --mpl-generate-summary=html \
--mpl-results-path="windrose_test_output-${{ matrix.os }}-${{ matrix.python-version }}"
- name: Store mpl-results
uses: actions/upload-artifact@v4
Copy link
Collaborator

Choose a reason for hiding this comment

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

Nice!

if: failure()
with:
name: "windrose_test_output-${{ matrix.os }}-${{ matrix.python-version }}"
path: "windrose_test_output-${{ matrix.os }}-${{ matrix.python-version }}"
retention-days: 1
5 changes: 5 additions & 0 deletions CHANGELOG.md
Expand Up @@ -5,6 +5,11 @@
All notable changes to this code base will be documented in this file,
in every released version.

### Version 1.x.x (unreleased)

## What's Changed
* Fix issue where sometimes the plot sectors showed a straight line instead of a curved one (#137)

### Version 1.7.0

## What's Changed
Expand Down
Binary file modified tests/output/df/test_bar.png
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Binary file modified tests/output/df/test_box.png
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Binary file modified tests/output/func/test_wrbox.png
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Binary file modified tests/output/oo/test_windrose_stacked_histogram_normed.png
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Binary file modified tests/output/test_bar_from_factory.png
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
4 changes: 4 additions & 0 deletions windrose/windrose.py
Expand Up @@ -642,6 +642,8 @@ def bar(self, direction, var, **kwargs):
zorder=zorder,
**kwargs,
)
# needed so the the line of the rectangle becomes curved
patch.get_path()._interpolation_steps = 100
Copy link
Collaborator

Choose a reason for hiding this comment

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

Ultimately, if I understand this correctly, this is a hack b/ it is using a private method to please our eyes with something that did not looked curved due to low resolution but was "correct." With that said, I do prefer this behavior even if it is prone to break with future changes in mpl.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I 100% agree on that being not optimal and originally private, but since it's even in the docs I think that's the only way to get there?

I still do think a straight line is technically incorrect in a polar projection.

Copy link
Collaborator

Choose a reason for hiding this comment

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

I still do think a straight line is technically incorrect in a polar projection.

For our eyes? Yes. I agree. But in theory the "data" is correct otherwise the higher interpolation steps would still produce a straight line, right?

Anyway, I did not mean that this was the problem as for a graph what we see is the important part. I think that mpl is probably choosing non-ideals defaults for us in this case.

self.add_patch(patch)
if j == 0:
self.patches_list.append(patch)
Expand Down Expand Up @@ -718,6 +720,8 @@ def box(self, direction, var, **kwargs):
zorder=zorder,
**kwargs,
)
# needed so the the line of the rectangle becomes curved
patch.get_path()._interpolation_steps = 100
self.add_patch(patch)
if j == 0:
self.patches_list.append(patch)
Expand Down