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

rename / redefine the parameters for the healpix index #34

Closed
keewis opened this issue Dec 4, 2023 · 1 comment · Fixed by #39
Closed

rename / redefine the parameters for the healpix index #34

keewis opened this issue Dec 4, 2023 · 1 comment · Fixed by #39

Comments

@keewis
Copy link
Collaborator

keewis commented Dec 4, 2023

We currently use rot_latlon, nside, and nest as grid parameters for healpix (mostly because those were inspired by healpy). However, I'd like to propose to a few changes:

  • nside: nside describes the number of subdivisions per side, and is always a power of two. That means that by taking the base-2 logarithm of nside we get a new parameter with a definition that is much closer to the resolution of other DGGS. New names could be order (from healpy), depth (from cdshealpix), level, or indeed resolution.
  • nest: In healpy, this is used to switch between the nested and ring indexing schemes. However, those are not the only ones in use, the unique scheme also exists. As such, a boolean parameter does not seem to be a good idea. Instead, I'd use a string: indexing_scheme="nested", indexing_scheme="ring", or indexing_scheme="unique".
  • rot_latlon: I would call this just rotation (proj calls this rot_xy to be consistent with the projections), where just like with healpy / cdshealpix the order would always be longitude then latitude. The values would still be specified as a list / 1D array. For example: rotation=[0, 30].

What do you think?

@benbovy
Copy link
Member

benbovy commented Dec 12, 2023

Those changes all make sense to me.

#35 would make it easier supporting both nside and resolution as inputs and validate the value.

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 a pull request may close this issue.

2 participants