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

Parse sub crs #30

Merged
merged 2 commits into from
Oct 15, 2024
Merged

Parse sub crs #30

merged 2 commits into from
Oct 15, 2024

Conversation

ftoromanoff
Copy link
Contributor

Most Crs are based on other crs. (ie PROJC use GEOCRS)
In this PR I propose to treat each sub crs as a CRS to not only parse the main crs but laso the subcrs.

The idea behind this would be in case of COMP_CS to be able to directly extract the sub crs (usually the PROJCS and the VERT_CS) instead of spliting the string wk.

wkt.build.js Outdated
Comment on lines 300 to 303
if (wkt.AUTHORITY) {
var authority = Object.keys(wkt.AUTHORITY)[0];
wkt.title = `${authority}:${wkt.AUTHORITY[authority]}`;
}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This change is unrelated, and wkt.build.js should not be manually modified, because it is auto-built.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Commit removed. It was the one for my other PR.

About the wkt.build.js change. I thought I had to run 'npm run build' before posting the PR, The changes on that file are from that run...

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I need to update the README and add that file to .gitignore. Sorry that the instructions are currently not clear about that.

Copy link
Member

@ahocevar ahocevar left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

To understand better what you're trying to achieve, it would be good if you could describe how you'd like to use the API to get components of a compound CRS.

The cleanWKT method won't make sense for e.g. parsing a datum. It might indeed make sense to parse components of a compound crs, but only if the component is not a vertical crs. And there is a risk of overwriting keys in the root object. Maybe a separate method to dissect compound CRSs would be better?

@ftoromanoff
Copy link
Contributor Author

ftoromanoff commented Oct 10, 2024

My idea was that in the case of a compound crs, we have several children crs from the parent compound crs.
So when using a wkt instead of spliting in the wkt string the part where we get information about the PROJCS and the VERT_CS to use proj4.defs on these why not directly get the proj.defs of the sub crs.

I noticed from the code that if we parse a GEOGCS we will get wkt.projName = 'longlat'.
But when we parse a PROJCS which is compose of a GEOCS and other information, the GEOCS part won't have the projName set to 'longlat'. Thus my idea was to recursively apply cleanWKT on all sub crs.

But indeed, I should probably have removed all the DATUM from getting in the boucle. (I initially copy all case from process.js from line 81 to 101).

About the risk of overwriting keys, I might be wrong, but i don't see it happening as we only add properties on the sub objects we are cleaning.

@ahocevar
Copy link
Member

ahocevar commented Oct 10, 2024

It seems that what you're looking for is the structure of the WKT as objects. So my suggestion would be to export the parseString() function from parser.js, and let the default export also work with already parsed objects. Then, assuming the title is populated as suggested in #29, you could do something like

import {parseString}, toDef from 'wkt-parser';
import proj4 from 'proj4';

const compound = parseString('COMPD_CS["SRGI2013 + INAGeoid2020 v1 height",GEOGCS["SRGI2013",DATUM["Sistem_Referensi_Geospasial_Indonesia_2013",SPHEROID["WGS 84",6378137,298.257223563,AUTHORITY["EPSG","7030"]],AUTHORITY["EPSG","1293"]],PRIMEM["Greenwich",0,AUTHORITY["EPSG","8901"]],UNIT["degree",0.0174532925199433,AUTHORITY["EPSG","9122"]],AUTHORITY["EPSG","9470"]],VERT_CS["INAGeoid2020 v1 height",VERT_DATUM["Indonesian Geoid 2020 version 1",2005,AUTHORITY["EPSG","1294"]],UNIT["metre",1,AUTHORITY["EPSG","9001"]],AXIS["Gravity-related height",UP],AUTHORITY["EPSG","9471"]],AUTHORITY["EPSG","9529"]]');

if (compound.GEOGCS && compound.GEOGCS.title) {
  const def = toDef(compound.GEOGCS);
  proj4.defs(def.title, def);
}

@ftoromanoff
Copy link
Contributor Author

ftoromanoff commented Oct 11, 2024

Indeed, what I want is to be able to extract part of wkt object (a wkt string parsed with parseString()) and use it to define a new projection in proj4 (using proj4.defs()).

(If I get it right wkt-parser.parseString() take a string and return an ARRAY and cleanWKT() take an object and return the same object with new properties, thus in your code the const 'compound' will be an ARRAY and not an object, the convertion from ARRAY to Object is done by sExpr())

So in your case it seems that we only need to have toDef() = cleanWKT()

And to do that, we have two options:

  1. export cleanWKT()
    code will then be
import { cleanWKT }, parse from 'wkt-parser';
import proj4 from 'proj4';

const compound = parse('COMPD_CS["SRGI2013 + INAGeoid2020 v1 height",GEOGCS["SRGI2013",DATUM["Sistem_Referensi_Geospasial_Indonesia_2013",SPHEROID["WGS 84",6378137,298.257223563,AUTHORITY["EPSG","7030"]],AUTHORITY["EPSG","1293"]],PRIMEM["Greenwich",0,AUTHORITY["EPSG","8901"]],UNIT["degree",0.0174532925199433,AUTHORITY["EPSG","9122"]],AUTHORITY["EPSG","9470"]],VERT_CS["INAGeoid2020 v1 height",VERT_DATUM["Indonesian Geoid 2020 version 1",2005,AUTHORITY["EPSG","1294"]],UNIT["metre",1,AUTHORITY["EPSG","9001"]],AXIS["Gravity-related height",UP],AUTHORITY["EPSG","9471"]],AUTHORITY["EPSG","9529"]]');

if (compound.GEOGCS && compound.GEOGCS.title) {
  const def = cleanWKT(compound.GEOGCS);
  proj4.defs(def.title, def);
}
  1. Adding the missing properties of the sub CRS like object when we call cleanWKT() the first time we parse the wkt.
    And in this case that's exacmty what this PR was for. But maybe I'm elarging the 'CRS like object' too wildely.

I fell like the second solution will be cleaner in terme of API

@ahocevar
Copy link
Member

ahocevar commented Oct 11, 2024

The first option is better. Because what you're calling subCRS can be many things that cleanWKT cannot work on in a meaningful way, things that proj4 does not understand, also things that aren't even a CRS.

@ahocevar
Copy link
Member

Of course the cleanest thing, but more complicated, would be something in between your options 1 and 2 - only add the (more or less proj4 specific) properties that the poorly named cleanWKT option adds for object types where it makes sense. GEOGCS, GEOCCS and a few more - not sure which ones exactly.

@ahocevar
Copy link
Member

ahocevar commented Oct 11, 2024

If proj4js is correct, then the supported object types are

https://github.com/proj4js/proj4js/blob/1da4ed0b865d0fcb51c136090569210cdcc9019e/lib/parseCode.js#L11

That would indeed mean to use this pull request almost as-is, i.e. with recursive cleanWKT, and calling cleanWKT also on the root level only if the object is one of the above.

@ftoromanoff ftoromanoff force-pushed the parseSubCrs branch 2 times, most recently from 0e7ca3c to 80c9080 Compare October 11, 2024 16:33
@ftoromanoff
Copy link
Contributor Author

I did a first trial to apply cleanWKT() ( which I renamed setPropertiesFromWKT()) recurively on all level (including root) if the are in the proj4list.

PS: I saw that several tests crashed, and i guess it's linked to the renaming of the function. And I wont be able to work on that before monday.

@ftoromanoff ftoromanoff force-pushed the parseSubCrs branch 3 times, most recently from c211c9a to ea5c72d Compare October 14, 2024 10:03
@ftoromanoff
Copy link
Contributor Author

I did fix the tests, As nome we 'clean' the 'sub'-crs we had to add the new properties to them, that we previously skipped.

Now if we parse a GEOGCS and a PROJCS based on the same GEOCS, we will in both case get the same objet.

@ahocevar ahocevar merged commit e1c270c into proj4js:main Oct 15, 2024
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants