Opened 5 years ago

Closed 22 months ago

#2417 closed enhancement (fixed)

r.profile should report distance in location units

Reported by: annakrat Owned by: grass-dev@…
Priority: major Milestone: 7.4.1
Component: Projections/Datums Version: svn-trunk
Keywords: r.profile, units Cc:
CPU: Unspecified Platform: All

Description

r.profile reports the length in meters no matter which units the current location uses. It would be better if it could report it in the current units. This ticket is created based on #1787. Once this is changed, certain changes must be reverted in GUI profile tool (r61910, r61269).

Attachments (4)

r_profile_location_units.diff (614 bytes) - added by mlennert 5 years ago.
patch to enable location units
r.profile.diff (4.8 KB) - added by annakrat 5 years ago.
New patch for r.profile
libgis_US_USFEET.diff (3.9 KB) - added by neteler 5 years ago.
Support for US feet (incomplete)
libgis_US_USFEET2.diff (4.2 KB) - added by annakrat 5 years ago.
working patch for us feet

Download all attachments as: .zip

Change History (23)

Changed 5 years ago by mlennert

patch to enable location units

comment:1 in reply to:  description ; Changed 5 years ago by mlennert

Replying to annakrat:

r.profile reports the length in meters no matter which units the current location uses. It would be better if it could report it in the current units.

The attached patch is a very simple attempt to use location units. I don't have time right now to revert the GUI profile tool changes to check whether this solves the problem, but at least with this patch distances are in location units.

The question is whether this should be the default behaviour of r.profile, or whether this should be only activated when a flag is set.

Moritz

Changed 5 years ago by annakrat

Attachment: r.profile.diff added

New patch for r.profile

comment:2 in reply to:  1 ; Changed 5 years ago by annakrat

Replying to mlennert:

Replying to annakrat:

r.profile reports the length in meters no matter which units the current location uses. It would be better if it could report it in the current units.

The attached patch is a very simple attempt to use location units. I don't have time right now to revert the GUI profile tool changes to check whether this solves the problem, but at least with this patch distances are in location units.

Thanks for looking into this. The patch seems to work. I got confused from the handling of units in the library which is completely weird, and I thought the conversion is not working properly but it's fortunately computed correctly. I improved the patch. The only problem is the name of the units which is in case of Foot_US not recognized and G_database_unit_name gives just unit. As a result, r.profile now prints something like this

Output Format:
[Along Track Dist. (units)] [Elevation]
Approx. transect length [205.092833] units
...

but this is a different problem.

The question is whether this should be the default behaviour of r.profile, or whether this should be only activated when a flag is set.

In my opinion, in most cases you want the current units. If you think the flag would be useful, it should switch to meters and by default (unset flag) use current units. Anyone has any opinion on adding this flag?

comment:3 in reply to:  2 Changed 5 years ago by mlennert

Replying to annakrat:

The only problem is the name of the units which is in case of Foot_US not recognized and G_database_unit_name gives just unit. As a result, r.profile now prints something like this

Output Format:
[Along Track Dist. (units)] [Elevation]
Approx. transect length [205.092833] units
...

but this is a different problem.

It seems to me that it should be possible to get the correct units name from the PROJ_UNITS file, but I just can't find a library function to do just that. I'm sure it must exist...

Moritz

comment:4 Changed 5 years ago by annakrat

I implemented the -m flag in r62035. In case of latlon, meters are used always. I reverted the changes in GUI.

comment:5 in reply to:  4 Changed 5 years ago by mlennert

Replying to annakrat:

I implemented the -m flag in r62035. In case of latlon, meters are used always. I reverted the changes in GUI.

Great, thanks !

This still leaves the issue of how to get the unit names if they are outside the standard. As they are correct in PROJ_UNITS there should be a way to get them from there.

Moritz

comment:6 in reply to:  4 ; Changed 5 years ago by marisn

Replying to annakrat:

I implemented the -m flag in r62035. In case of latlon, meters are used always. I reverted the changes in GUI.

If changing module parameters, wouldn't it be better to implement it as a choice option with appropriate parser standard option chooser? Default could be location units. Then the user could decide, if he wants to have metres, feet, etc. Just a question not a request.

comment:7 Changed 5 years ago by neteler

Milestone: 7.1.07.0.0
Priority: normalmajor

At least in California (http://maps.cityofdavis.org/library/, Street Centerline, http://epsg.io/102642) as well as some North Carolina .prj definitions with

  • unit: Foot_US
  • units: Foot_USs

still exist, hence should be supported as well.

I checked and found some older discussion:

Re: U_FEET versus US_USFEET, there has been discussion in the past:

I have created a patch (attached) which introduces US_USFEET as suggested years ago by Glynn. It contains one TODO where I don't know how to distinguish the special State Plane feet case. How to do that properly? with PROJECTION_SP_USF?

TODO: GS_geodistance() in ogsf/GS_util.c comes with hardcoded values! Instead G_meters_to_units_factor() should be used, stuff to be merged.

Changed 5 years ago by neteler

Attachment: libgis_US_USFEET.diff added

Support for US feet (incomplete)

Changed 5 years ago by annakrat

Attachment: libgis_US_USFEET2.diff added

working patch for us feet

comment:8 in reply to:  7 Changed 5 years ago by annakrat

Replying to neteler:

I have created a patch (attached) which introduces US_USFEET as suggested years ago by Glynn. It contains one TODO where I don't know how to distinguish the special State Plane feet case. How to do that properly? with PROJECTION_SP_USF?

It seems that the PROJECTION_SP is not recognized anyway see GPJ_osr_to_grass. I attached a fixed patch, r.profile seems to report units now correctly (tested with nc_spf location).

TODO: GS_geodistance() in ogsf/GS_util.c comes with hardcoded values! Instead G_meters_to_units_factor() should be used, stuff to be merged.

This function is not used (but it should be merged anyway).

If there are no objections, I will commit it.

comment:9 in reply to:  6 Changed 5 years ago by annakrat

Replying to marisn:

Replying to annakrat:

I implemented the -m flag in r62035. In case of latlon, meters are used always. I reverted the changes in GUI.

If changing module parameters, wouldn't it be better to implement it as a choice option with appropriate parser standard option chooser? Default could be location units. Then the user could decide, if he wants to have metres, feet, etc. Just a question not a request.

Yes, it was not so difficult so I added this option in r62046 (and removed the flag).

comment:10 Changed 5 years ago by neteler

Thanks for fixing my proposed patch. Since PROJECTION_SP is not recognized but treated as PROJECTION_OTHER, I suggest to remove the "case PROJECTION_SP" entirely in order to simplify the code.

Please commit then.

comment:11 Changed 5 years ago by annakrat

Support for feet added in r62060 and PROJECTION_SP removed in r62061. Hopefully we can close this after backporting (will wait a couple of days).

comment:12 Changed 5 years ago by neteler

While we are at it, I checked the entire source code for more "private" feet definitions, there are some to be fixed:

[neteler@pgis_north grass71]$ find . -type f | xargs grep 3048 | grep -v 'pristine'
./general/g.setproj/README:	feet:foot:0.3048
./general/g.setproj/main.c:		    G_set_key_value("meters", "0.30480060960121920243",
./general/g.setproj/main.c:		    G_set_key_value("meters", "0.3048", in_unit_keys);
./raster/r.ros/spot_dist.c:    h0 = 0.3048 * (1.055 * sqrt(E));
./raster/r.watershed/ram/Gwater.h:#define METER_TO_FOOT		(1 / 0.3048)
./raster/r.watershed/seg/Gwater.h:#define METER_TO_FOOT		(1 / 0.3048)
./raster/r.shaded.relief/main.c:	    /* 1 international foot = 0.3048 meters */
./raster/r.shaded.relief/main.c:	    scale = 1. / 0.3048;
./raster/r.buffer/distance.h:#define FEET_TO_METERS 0.3048
./ps/ps.map/distance.h:#define FEET_TO_METERS 0.3048
./scripts/r.buffer.lowmem/r.buffer.lowmem.py:    'feet': 0.3048,
./vector/v.split/main.c:#define FROM_FEET        0.3048

comment:13 Changed 5 years ago by annakrat

Changes in library, r.profile and gui backported in r62177, r62178.

comment:14 Changed 4 years ago by martinl

Milestone: 7.0.07.0.5

comment:15 Changed 3 years ago by martinl

Milestone: 7.0.57.3.0

comment:16 Changed 3 years ago by martinl

Milestone: 7.3.07.4.0

Milestone renamed

comment:17 Changed 22 months ago by martinl

What is missing for closing the issue?

comment:18 Changed 22 months ago by neteler

Milestone: 7.4.07.4.1

Ticket retargeted after milestone closed

comment:19 Changed 22 months ago by annakrat

Resolution: fixed
Status: newclosed
Note: See TracTickets for help on using tickets.