search

michitaro / healpix

An implementation of HEALPix in JavaScript / TypeScript
MIT License
13 stars 4 forks source link

Add ring2fxy #13

Closed cdeil closed 6 years ago

cdeil commented 6 years ago

This PR adds ring2fxy and calls it from ring2nest. I think that's easier to understand, making it clear that his package always goes via fxy.

There is a second commit, changing the return in tu2fxy to order fxy like in all other functions. Apparently there was no caller relying on this, at least no test is failing.

Overall I wonder if we should create V3 and FXY objects when returning things? Note how vec2ang shows a V3 object going in, but ang2vec doesn't show one coming out, just "Object". @michitaro - Do you think we should make a change here, or keep as is e.g. for performance reasons?

michitaro commented 6 years ago

Thank you for the PR.

This PR adds ring2fxy and calls it from ring2nest. I think that's easier to understand, making it clear that his package always goes via fxy.

I agree with that.

fxy is an internal thing, so I unexported it. https://github.com/michitaro/healpix/pull/13/commits/61267d82645c3158db8d1884ebb110da499dce35#diff-f41e9d04a45c83f3b6f6e630f10117feR69

cdeil commented 6 years ago

fxy is an internal thing, so I unexported it.

I think there could be value in exposing more from your library.

E.g. note that your za2tu and tu2za, i.e. the spherical projection is useful to have (see e.g. https://github.com/substack/healpix or WCSLib).

XY is used e.g. in HiPS (see Fig 4 in PDF here). There it's XY within a tile, but that would be easy to compute I think if one has the global XY within the base pixel.

So overall I'm +1 to expose more functions, to make them appear in the docs and easily accessible.

michitaro commented 6 years ago

OK, I understand some internal functions are useful for users. To export them, we have to clarify notations or have some document.

cdeil commented 6 years ago

Like I said in #11, I think we should start with that list and make it more visible in the docs.

I'm not familiar with the TS docs system. Should we put this in the README? Or are there "docstrings" we can put at the top of the file and then it will appear at https://michitaro.github.io/healpix/typedoc/modules/_index_.html ?

I'd be happy to help fill the documentation a bit, to continue to learn about HEALPix.

michitaro commented 6 years ago

I really appreciate your help. I added notations on typedoc.

michitaro commented 6 years ago

We can add comments on typedoc something like this.