Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Temporal CRS Unit Conversion Factor Unhandled #2275

Closed
snowman2 opened this issue Jun 28, 2020 · 9 comments
Closed

Temporal CRS Unit Conversion Factor Unhandled #2275

snowman2 opened this issue Jun 28, 2020 · 9 comments
Labels

Comments

@snowman2
Copy link
Contributor

snowman2 commented Jun 28, 2020

Example of problem

projinfo  'TIMECRS[“GPS milliseconds”,TDATUM[“GPS time origin”,TIMEORIGIN[1980-01-01T00:00:00.0Z]],CS[TemporalCount,1],AXIS["(T)“,future,TIMEUNIT[”millisecond (ms)",0.001]]]'
input string: parsing of user string failed: unhandled axis direction: 0.001

Problem description

WKT string from example 2: http://docs.opengeospatial.org/is/18-010r7/18-010r7.html#105

Expected Output

No error.

Environment Information

  • PROJ version - latest master
  • Operation System Information - Ubuntu 18

Installation method

  • From source
@snowman2 snowman2 added the bug label Jun 28, 2020
@rouault
Copy link
Member

rouault commented Jun 28, 2020

Actually, this is more a bug of the example than PROJ.

The examples in the WKT standard tend to use Unicode printing open quote (“) and Unicode printing closing quote (”), but this isn't even allowed by the standard. Probably an artifact of the standard being written in a text editor that does the substitution for ASCII double-quote automatically. So the PROJ parser nominally supports ASCII double quote, and as a convenience a correct balancing of Unicode printing open quote (“) and Unicode printing closing quote (”), but in that example the AXIS name starts with ASCII double quote, which is terminated after the (ms) of TIMEUNIT.

For me, this is a wont fix situation

@rouault
Copy link
Member

rouault commented Jun 28, 2020

Note: I've raised the issue to the OGC CRS WG mailing list

@RogerLott
Copy link

I agree, and that was certainly the intention, in both 2015 and 2019 versions. As editor of OGC 18-010r7 I will look at fixing the source Word document. I guess there could be similar problem with other characters that appear in multiple Unicode planes indistinuishable to humans reading a text document. Any easy way of checking for this in a text document?

But in the comment above @Roualt said

For me, this is a wont fix situation
What did you mean by that?

@rouault
Copy link
Member

rouault commented Jun 28, 2020

Any easy way of checking for this in a text document?

With really a text (.txt) document, that is not a Word one, yes, with Unix utilities it is easy:

$ echo 'TIMECRS[“GPS milliseconds”,TDATUM[“GPS time origin”,TIMEORIGIN[1980-01-01T00:00:00.0Z]],CS[TemporalCount,1],AXIS["(T)“,future,TIMEUNIT[”millisecond (ms)",0.001]]]' > out.txt

$ file out.txt
out.txt: UTF-8 Unicode text

vs

$ echo 'TIMECRS["GPS milliseconds",TDATUM["GPS time origin",TIMEORIGIN[1980-01-01T00:00:00.0Z]],CS[TemporalCount,1],AXIS["(T)",future,TIMEUNIT["millisecond (ms)",0.001]]]' > out.txt

$ file out.txt
out.txt: ASCII text

For me, this is a wont fix situation
What did you mean by that?

I just mean that I don't think there is anything to change in PROJ regarding this. The issue is with the WKT provided as input, not with the software implementation

@desruisseaux
Copy link
Contributor

I think Even means that PROJ current behaviour should not be modified (accept matching quotes, but report mixing of ASCII quotes with Unicode quotes as an error). Apache SIS has the same policy.

Alternatively, the examples could also be copied in plain text files, one file per WKT, and committed on an OGC Github.

@desruisseaux
Copy link
Contributor

For information, examples from ISO 19162:2015 are available as GeoAPI tests executable by PROJ using the JNI bindings (I did not had the time to upgrade to ISO 19162:2019 yet). I have just executed the tests. Many of them pass, the following ones may need investigation:

  • testTemporal() (from OGC 12-063r5 §14.4): PROJ said "Missing EDATUM / ENGINEERINGDATUM node"
  • testCompoundWithTime() (from OGC 12-063r5 §16.2 example 3): PROJ said "unexpected TIMEUNIT"
  • testDerivedEngineeringFromGeodetic() (from OGC 12-063r5 §15.5.2 example 2): PROJ said "Missing EDATUM / ENGINEERINGDATUM node"
  • testDerivedEngineeringFromProjected() (from OGC 12-063r5 §15.5.2 example 1): PROJ said "Missing EDATUM / ENGINEERINGDATUM node"
  • testProjectedWithImplicitParameterUnits() (from OGC 12-063r5 §9.5 example 3): PROJ said "Parsing error : syntax error, unexpected PRIMEM, expecting ID"

rouault added a commit to rouault/PROJ that referenced this issue Jun 28, 2020
…se TIMEUNIT is at the CS level, and not inside

Adresses testTemporal() and testCompoundWithTime() cases of
OSGeo#2275 (comment)
@rouault
Copy link
Member

rouault commented Jun 28, 2020

  • testTemporal() and testCompoundWithTime() warnings addressed by WKT parser: do not raise warning when parsing a WKT2:2015 TIMECRS whose TIMEUNIT is at the CS level, and not inside #2276
  • testDerivedEngineeringFromGeodetic() and testDerivedEngineeringFromProjected(): those constructs are not supported by PROJ. This is the remark of WKT:2019 "Subclause 14.6, derived engineering CRS (ISO 19162:2015, 15.5): This was incorrectly described in 19162:2015. The type of base CRS as either projected or geographic or engineering allowed in 19162:2015 was wrong. It has been entirely remodelled in two distinct parts: a derived engineering CRS is to have a base CRS of type engineering (base CRS of projected or geodetic no longer permitted); derived projected CRS is to have a base CRS of type projected and hence a geodetic datum."
  • testProjectedWithImplicitParameterUnits(): this WKT construct is not strictly compliant with the WKT:2015 BNF. The PRIMEM shoud be before ANGLEUNIT, and another issue is the ID of the CONVERSION is just after the name, instead of being at the end. The C++ WKTParser is in strict mode by default (that it it does a pre-check using a parser closely aligned with the BNF, whereas the ingestion of the WKT nodes to C++ objects is done in a laxer way). This can be changed by calling the setStrict(false) method. The C API uses non-strict mode.

rouault added a commit that referenced this issue Jul 1, 2020
…se TIMEUNIT is at the CS level, and not inside (#2276)

Adresses testTemporal() and testCompoundWithTime() cases of
#2275 (comment)
@RogerLott
Copy link

The problem with non-ASCII characters in OGC 18-010r7 is in the html document at http://docs.opengeospatial.org/is/18-010r7/18-010r7.html. The Word version at https://portal.opengeospatial.org/files/18-010r7/docx and the pdf at https://portal.opengeospatial.org/files/18-010r7 do not appear to have the problem. It appears to have been caused in the Pandoc conversion. The html document will be corrected - I don't have a timeframe.

@RogerLott
Copy link

Hopefully the html document now fixed.

I suggest that this issue could now be closed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

4 participants