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 auto-centering of altitude colormaps. #27

Closed
1 of 3 tasks
juseg opened this issue Oct 31, 2022 · 2 comments · Fixed by #36
Closed
1 of 3 tasks

Fix auto-centering of altitude colormaps. #27

juseg opened this issue Oct 31, 2022 · 2 comments · Fixed by #36
Assignees
Labels
documentation Improvements or additions to documentation
Milestone

Comments

@juseg
Copy link
Owner

juseg commented Oct 31, 2022

I realize the documentation are dotted with center=False. This is because xarray defaults to centering any image plot where data span negative and positive values, however this is not really appropriate for glacier-depressed bedrock elevation typically ranging from slightly below sea level to the mountain tops. I can think of two ways to fix this: use sealevel=-120 and possibly the 'Elevational' instead of 'Topographic' colormap, or changing bedrock_altitude() and friends' default behaviour.

  • Implement better defaults for altitude?
  • Remove center=False in docs.
  • Doc changes in whatsnew.
@juseg juseg added this to the v0.2.1 milestone Oct 31, 2022
@juseg juseg self-assigned this Oct 31, 2022
@juseg
Copy link
Owner Author

juseg commented Nov 30, 2022

I am backtracking on this. I think I want to keep hyoga as consistent as possible with xarray. The center kwarg is not the most intuitive though. Besides I realized that sealevel=sealevel is redundant with center=sealevel. So I am planning to:

  • replace center=False with vmin=0 or vmin=-120 in docs.
  • deprecate sealevel argument to bedrock_altitude.

@juseg
Copy link
Owner Author

juseg commented Nov 30, 2022

Using vmin=-120 alone causes xarray to center the cmap between -120 and +120, so I'll stick to center=False in that case but:

  • add a note to explain center=False in docs.

@juseg juseg added documentation Improvements or additions to documentation and removed enhancement New feature or request labels Nov 30, 2022
@juseg juseg linked a pull request Nov 30, 2022 that will close this issue
@juseg juseg closed this as completed in #36 Nov 30, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation Improvements or additions to documentation
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant