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

Colorbar example for heatmap #1736

Merged
merged 7 commits into from Apr 6, 2022
Merged

Colorbar example for heatmap #1736

merged 7 commits into from Apr 6, 2022

Conversation

KronosTheLate
Copy link
Contributor

Description

This PR adds documentation to the page on heatmap, specifically how to make a colorbar.

It also removes the previous final section Colors, as it

  1. Seems to have an outdated reference, and
  2. had nothing else, effectively contributing nothing but noise.

The final section, color, refered to the "previous example", which did not do anything with colors. So it had an outdated reference, and did not add anything other than this outdated reference. I found it to be only noise, and therefore feel it is better to remove it.
@KronosTheLate
Copy link
Contributor Author

Credit also goes to @MaPagh for pointing out this lack of documentation, so that it could be added.

@KronosTheLate
Copy link
Contributor Author

Friendly bump

Copy link
Member

@SimonDanisch SimonDanisch left a comment

Choose a reason for hiding this comment

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

looks good to me :)
@jkrumbiegel can we merge this, or would you document the usage of Colorbar differently?

zs1 = [sin(x*y) for x in xs, y in ys]
zs2 = [2sin(x*y) for x in xs, y in ys]

my_limits = extrema(hcat(zs1, zs2)) # Get maximum and minimum for all z-values
Copy link
Collaborator

Choose a reason for hiding this comment

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

I don't like this too much because I can see the hcat breaking on different datasets already, where the matrices don't match. I haven't been able to come up with a really nice and short "cumulative extrema of multiple arrays" expression though that's easy to understand. Maybe it's fine just specifying -2, 2 here manually as we know that from the 2sin expression.

Copy link
Member

Choose a reason for hiding this comment

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

How about extrema(extrema.((zs1, zs2)))

Copy link
Collaborator

Choose a reason for hiding this comment

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

That makes a tuple of tuples

Copy link
Member

Choose a reason for hiding this comment

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

Ah yeah...
@KronosTheLate, should we just use (-2, 2)... I think that's fine here, since we're not trying to show how to get the extrema from the values ;)

Copy link
Collaborator

Choose a reason for hiding this comment

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

I think that's fair, it's reasonably obvious with the sine (actually it can even look better because if the limits go from -1.99123 to 1.992852 then the colorbar ticks don't have -2 and 2)

Copy link
Member

Choose a reason for hiding this comment

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

Ok, I'll edited it... @KronosTheLate I think we can merge it now, if you don't have objections

Copy link
Contributor Author

Choose a reason for hiding this comment

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

No objections - extrema of extrema is besides the point of this documentation, so manual limits, though inelegant in some ways, seems like a great solution for this documentation ^_^

docs/examples/plotting_functions/heatmap.md Outdated Show resolved Hide resolved
@@ -128,7 +128,7 @@ ys = range(0, 2π, length=100)
zs1 = [sin(x*y) for x in xs, y in ys]
zs2 = [2sin(x*y) for x in xs, y in ys]

my_limits = extrema(hcat(zs1, zs2)) # Get maximum and minimum for all z-values
my_limits = (-2, 2) # choose some good limits for sin / 2sin
Copy link
Collaborator

Choose a reason for hiding this comment

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

maybe not my_limits but joint_limits or something, and say # here we pick the limits manually for simplicity instead of computing them so that it's clear that that would be the normal course of action?

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 points. I agree with thoose changes.
The optimal thing would perhaps be a helper function that would find the extremas of any heatmap, contour or contour3D (I think those are the ones this would apply to?) in the given figure.

@SimonDanisch SimonDanisch merged commit d964e56 into MakieOrg:master Apr 6, 2022
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.

None yet

3 participants