Change withOrigin -> shiftOrigin for non-local WCS types#1085
Conversation
| the origin position the same way the current WCS treats (x,y) = (0,0). | ||
| This function creates a new WCS instance (always a non-local WCS) that shifts the | ||
| origin by the given amount. In other words, it treats the image position ``origin`` | ||
| the same way the current WCS treats (x,y) = (0,0). |
There was a problem hiding this comment.
The last statement in this paragraph confuses me. I think it is technically correct, but the language throws me. IDK if there is anything to do about this though and I don't have a good suggestion.
There was a problem hiding this comment.
I guess the point I was trying to make is that the "origin" of the current wcs doesn't have to be (0,0). It is if it's local, but not necessarily otherwise. So whatever real position on the image is currently called (0,0), the new wcs will call that position origin, the argument being passed to this function. I'm not sure what would be a better sentence to explain that, but if you can think of something that would make that clearer, I'm game to change it.
There was a problem hiding this comment.
Yeah I’ve got nothing. I’d say merge the pr and I can make another later to change things if I think I can say it more clearly.
| origin: The image coordinate position to use as the origin. | ||
| world_origin: The world coordinate position to use as the origin. Only valid if | ||
| origin: The image coordinate position to use for what is currently treated | ||
| as (0,0). |
There was a problem hiding this comment.
Ditto here for language but again I don't have a better suggestion.
rmjarvis
added
base classes
2 participants
This is a quick hit to address issue #1073.
For non-local WCS types, the name
withOriginis deprecated in favor ofshiftOrigin. The functionality hasn't changed, but the name withOrigin is only really appropriate for LocalWCS types. When the WCS already has a non-zero origin, then the action takes in really to shift the origin, not set a new value.I kept the name withOrigin for local WCS types where I think it is more intuitive and accurately describes the action taken. Both names are available for LocalWCS types and they are equivalent.
I also updated the docs a bit to hopefully make the action that is happening here somewhat clearer.