search

pynbody / genetIC

The GM initial conditions generator
GNU General Public License v3.0
21 stars 8 forks source link

Renaming of grid methods #21

Closed Martin-Rey closed 6 years ago

Martin-Rey commented 6 years ago

A bit of refactoring for clarity following the logic that grid cells can be accessed through an index, a coordinate (pixel coordinate) or a point (box coordinate).

Let me know if you think the names are unclear or if you have better propositions.

apontzen commented 6 years ago

Looks good overall... I wonder whether 'centroid' was more descriptive than 'point' though?

apontzen commented 6 years ago

(i.e. I am suggesting maybe the three words should be 'index', 'coordinate' and 'centroid' rather than 'index', 'coordinate' and 'point')

Martin-Rey commented 6 years ago

Centroid is very clear when starting from a cell, but there are many points that return the same cell, not just the centroid.

The clearer, though it makes names longer, would probably be index, PixelCoordinate, BoxCoordinate.

apontzen commented 6 years ago

I wonder whether the existing distinction between "centroid" (guaranteed to be the centroid) and "point" (any point in the cell) should be reinstated in that case. So you get an index from a point, but you get a centroid from an index. I realise there is then a lack of symmetry... but after all these are not really symmetric operations, so it seems accurate.

I don't find "pixel" descriptive, incidentally.

Martin-Rey commented 6 years ago

Sounds good to me.