Add support for deg, rad and grad in unitconvert (fixes #1052), and document that it supports numeric factors (refs #1053) #1054
Conversation
| ############################################################################### | ||
|
|
||
| In the table below all angular units supported by PROJ `unitconvert` are listed. | ||
| (since PROJ 5.2.0) |
There was a problem hiding this comment.
Please use .. versionadded:: 5.2.0 instead. Place it below the section header. That's consistent with the rest of the docs.
| units. | ||
| Horizontal input units. See :ref:`distance_units` and :ref:`angular_units` | ||
| for a list of available units. `<conversion_factor>` is the conversion factor | ||
| from the input unit to the International System unit. |
There was a problem hiding this comment.
Perhaps just say that it is the conversion factor to meters? Otherwise I would say that "SI unit" is closer to standard nomenclature. Also valid below.
There was a problem hiding this comment.
"conversion factor to meters"... if it is linear unit, otherwise it is to radian. I can add that precision
src/PJ_unitconvert.c
Outdated
|
|
||
|
|
||
| /***********************************************************************/ | ||
| static double getUnitConversionFactor(const char* name, |
There was a problem hiding this comment.
Good idea to add this function. Makes the code a bit easier to read. Small style nitpick: The rest of the functions use snake case so it looks a bit odd that this is named in camel case.
Non-sense, but should we try to protect against this ? |
rouault
force-pushed
the
add_angular_units_to_unitconvert
branch
from
e7626c0
to
7149043
Compare
I didn't think it would be necessary to protect against creation of non-sense pipelines but of course there were complaints that you could do just that. This is somewhat similar so maybe it is better to just take care of that in advance. It is not a top priority for me - it can also be handled by explaining the behaviour in the docs. |
|
|
||
| In the table below all angular units supported by PROJ `unitconvert` are listed. | ||
|
|
||
| .. versionadded:: 5.2.0 |
There was a problem hiding this comment.
If you put it right below the heading it is more clear that it is the whole section that was added in version 5.2.0
rouault
force-pushed
the
add_angular_units_to_unitconvert
branch
from
79e6904
to
d40ec8d
Compare
|
ok, I've added a consistency check for the unit types in xy_in vs xy_out and z_in vs z_out |
| @@ -583,6 +583,7 @@ enum deprecated_constants_for_now_dropped_analytical_factors { | |||
| #define PJD_ERR_ELLIPSOIDAL_UNSUPPORTED -56 | |||
| #define PJD_ERR_TOO_MANY_INITS -57 | |||
| #define PJD_ERR_INVALID_ARG -58 | |||
| #define PJD_ERR_INCONSISTENT_UNIT -59 | |||
There was a problem hiding this comment.
You should add a string representation in src/pj_strerrno.c as well
There was a problem hiding this comment.
Good point. I've added a comment at the end of the enumeration to update pj_strerrno.c AND pj_transform.c as well (and I've sanitized the access to the transient_error array in it)
|
Add the string representation of the new error code and I am happy with this. It's a nice new feature to have! |
…and document that it supports numeric factors (refs OSGeo#1053)
rouault
force-pushed
the
add_angular_units_to_unitconvert
branch
from
d40ec8d
to
5411bd3
Compare
No description provided.