Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Fix rangebreaks on heatmap with 2d z array #4821

Merged
merged 6 commits into from
May 11, 2020

Conversation

archmoj
Copy link
Contributor

@archmoj archmoj commented May 7, 2020

Fixes: #4818 | demo: before vs after.

@plotly/plotly_js

@archmoj archmoj added bug something broken status: reviewable labels May 7, 2020
@archmoj archmoj added this to the v1.54.2 milestone May 7, 2020
@archmoj archmoj requested a review from alexcjohnson May 7, 2020 22:08
"2020-01-02 17:40",
"2020-01-02 17:50",
"2020-01-02 18:00",
"2020-01-02 18:10"
Copy link
Collaborator

Choose a reason for hiding this comment

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

These tests include some removed data, which is good - but can we also include some data on the other side of the break, ie the next morning?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good call. Done in e2cf395.

@alexcjohnson
Copy link
Collaborator

Does this fix also support histogram2d with rangebreaks?

@archmoj
Copy link
Contributor Author

archmoj commented May 8, 2020

Does this fix also support histogram2d with rangebreaks?

Not sure about that one. Is there any problem you noticed with histogram2d and rangebreaks?

@alexcjohnson
Copy link
Collaborator

I haven't tried histogram2d with rangebreaks but I know it shares a lot of code with heatmap so seems natural to address them together.

- remove duplicate points from heatmap mocks
@archmoj
Copy link
Contributor Author

archmoj commented May 11, 2020

I added tests for contour in a7676bd.
The histogram2d case is tracked in #4825 amd it would be addressed in a separate PR.

Copy link
Collaborator

@alexcjohnson alexcjohnson left a comment

Choose a reason for hiding this comment

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

Nice, thanks for testing this out with contour as well! Agreed we can push histogram2d to another PR, I just wanted to ensure we didn't forget about it. 💃

@archmoj archmoj merged commit 5d35b78 into master May 11, 2020
@archmoj archmoj deleted the rangebreaks-heatmap-2d-z-array branch May 11, 2020 18:01
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
bug something broken
Projects
None yet
Development

Successfully merging this pull request may close these issues.

plot & hover problems of heatmap (case of 2d-z array) with rangebreaks
2 participants