search

astropy / pyregion

ds9 region parser for python
https://pyregion.readthedocs.io
MIT License
39 stars 38 forks source link

Document rot_wrt_axis parameter #84

Closed cdeil closed 7 years ago

cdeil commented 7 years ago

This is a minor cleanup to make rot_wrt_axis default keyword arguments bool instead of int, to make it clearer that this is a flag (and not e.g. a float rotation angle).

@leejjoon - OK?

coveralls commented 7 years ago

Coverage Status

Coverage remained the same at 50.714% when pulling 876cebf6287ee833a4b959a0b7c267b2d15ead89 on cdeil:rot_wrt_axis into 1304632aaa0e2098daaa17d16b177061fe44b1e0 on astropy:master.

leejjoon commented 7 years ago

'rot_wrt_axis' is from the PR (#34) by @mcara. And I guess he is using this with value of 2. So, this may affect his codes. And the name of the keyword means (I guess) "rotation with respect to axis" 1 or 2, so I guess 1 or 2 makes more sense that True/False. So, unless @mcara is supportive of this PR, I would rather leave it as is, but add an adequate documentation.

coveralls commented 7 years ago

Coverage Status

Coverage decreased (-0.05%) to 50.555% when pulling 7516fb2f8e24a15f59754088a0a6908583a3770b on cdeil:rot_wrt_axis into 34ebb82a37698dd1678ac9251455a932c2c1ada2 on astropy:master.

cdeil commented 7 years ago

@leejjoon @mcara - Thanks for the infos.

I've changed the commit here to document rot_wrt_axis as you explained:

        rot_wrt_axis : {1, 2}
            Use rotation with respect to axis 1 (X-axis) or axis 2 (Y-axis) and north.

I've also changed the use to raise a ValueError if the user passes something other than 1 or 2.