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

How to use the OFTStringList field type? #47

Open
bkeevil opened this issue Feb 25, 2021 · 5 comments
Open

How to use the OFTStringList field type? #47

bkeevil opened this issue Feb 25, 2021 · 5 comments
Labels
enhancement New feature or request help wanted Extra attention is needed

Comments

@bkeevil
Copy link

bkeevil commented Feb 25, 2021

I was trying something like:

layer.fields.add(new gdal.FieldDefn('candidates.names',gdal.OFTStringList));
...
var names = new Array();
document.candidates.forEach((c) => names.push(c.name));
feature.fields.set('candidates.names',names);

but feature.fields.set() won't accept an array of strings

What is the correct way to use the OFTStringList field type? I can't find any documentation or examples anywhere.

@yocontra
Copy link
Owner

yocontra commented Feb 26, 2021

This is the low level set call, I don't see anything here that would handle OFTStringList correctly:

Seems like the only supported types are numbers, strings, and null/undefined (to unset). I think it would be a good addition to make sure all types we support reading are also supported writing.

For your specific case you would need to check if the JS value is an array of strings, then cast/pass it to this https://gdal.org/api/ogrfeature_cpp.html#_CPPv4N10OGRFeature8SetFieldEPKcPPCKc - the lift should not be too bad if you want to send a PR.

@yocontra yocontra added enhancement New feature or request help wanted Extra attention is needed labels Feb 26, 2021
@mmomtchev
Copy link
Contributor

@contra I am not very sure if it is really useful - complex data types are considered exotic and while reading them is supported as they can be encountered - it is still best to not write them. You will either end producing incompatible output, or the driver will eventually stringify the object the way it sees it right.
Instead of using a string array property, use multiple string properties - and you will be sure that your GeoJSON stays compatible with everything else.

@bkeevil
Copy link
Author

bkeevil commented Feb 26, 2021

Are these datatypes part of some sort of standard?

@mmomtchev
Copy link
Contributor

Every format defines its own datatypes and then there is the GDAL level abstraction which tries to match them as closely as possible.

The scalar types and the string are universal, but the list types are much less so - for example given that GeoJSON is in fact a JSON, you can always try to squeeze an array (or worse, an object) into the properties object and while it will still be a valid JSON encoding, many GeoJSON tools won't parse it correctly. The GeoJSON RFC states that properties is an object - nothing more.

But still, as GDAL does support writing those list types, a PR for them would still get merged. It is just that there is a reason it was not made in the first place and unless you are going to be the only reading those files, you should probably avoid it.

If you are making a PR, just make sure to convert the UTF16 from V8 to UTF8 for GDAL. There are many examples in the code.

@bkeevil bkeevil closed this as completed Mar 20, 2021
@yocontra
Copy link
Owner

@bkeevil I'm going to leave this open since this is a feature request we want to implement

@yocontra yocontra reopened this Mar 20, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request help wanted Extra attention is needed
Projects
None yet
Development

No branches or pull requests

3 participants