astronomy-commons / hats

Hierarchical Progressive Survey Catalog
https://hats.readthedocs.io/
BSD 3-Clause "New" or "Revised" License
17 stars 5 forks source link

Healpix sorting behavior is ambiguous #364

Closed smcguire-cmu closed 1 week ago

smcguire-cmu commented 2 weeks ago

We allow sorting of the HealpixPixel class with the default data class behavior (first sorting by order, then pixel). I think it's probably more useful to have sorting by higher order pixel number, and I think there are a few instances in the codebase where this behavior is expected and pixel sorting is used incorrectly.

delucchi-cmu commented 2 weeks ago

We have a hats.get_pixel_argsort method that will sort by higher order. This is already used in a handful of places in LSDB. Are there specific locations where we're using an incorrect sorting?

smcguire-cmu commented 2 weeks ago

I think I was actually misreading the code where I thought this was a problem, sorry! I'm still not sure I particularly like the idea of HealpixPixels being ordered, that at least when I saw the uses of it after a while, I assumed the behavior was sorting by the healpix_29 of the pixels, not order then pixel. But that might just be a me problem, and if other people like it we can just close this issue.