Skip to content

Issue 103 - geojson_sf() with mixed logicals#104

Merged
dcooley merged 6 commits intoSymbolixAU:masterfrom
ateucher:issue170-mixed-logical
Nov 23, 2025
Merged

Issue 103 - geojson_sf() with mixed logicals#104
dcooley merged 6 commits intoSymbolixAU:masterfrom
ateucher:issue170-mixed-logical

Conversation

@ateucher
Copy link
Contributor

@ateucher ateucher commented Nov 7, 2025

This is an attempt to fix #103.

In get_property_types(), property_type ("True" or "False") essentially just reflects which boolean value was encountered first, but subsequent features can have a different type, so that type for a feature can be different from property_type for that property. Before, if a property had different types across features, it was immediately converted to "String". I added logic to recognize that "True" and "False" are the same logical type.

In fill_property_vectors(), it now allows both true and false values to be stored in the logical vector, regardless of whether the detected type was labeled as "True" or "False".

I added some new tests, which now pass (failed before), but definitely warrants checking my logic!

@dcooley
Copy link
Collaborator

dcooley commented Nov 8, 2025

Thanks @ateucher, looks good to me! Could you add (if you want to) a contributor role in the Description for yourself?

@ateucher
Copy link
Contributor Author

ateucher commented Nov 9, 2025

Great, thanks! Will do!

@ateucher
Copy link
Contributor Author

ateucher commented Nov 9, 2025

@dcooley do you want me to update the GitHub Actions workflows? They're not running as they use old unsupported workflows. I think r-lib/check-standard should be sufficient...

@ateucher ateucher changed the title Issue 170 - geojson_sf() with mixed logicals Issue 103 - geojson_sf() with mixed logicals Nov 9, 2025
@ateucher
Copy link
Contributor Author

Hi @dcooley just a little nudge on my question...

@dcooley
Copy link
Collaborator

dcooley commented Nov 20, 2025

sorry, am away on holiday; yes if you could get the actions working again that would be great

@ateucher
Copy link
Contributor Author

Ah, my apologies! Will do :)

@ateucher
Copy link
Contributor Author

ateucher commented Nov 20, 2025

I made some updates to the DESCRIPTION file, as installing the dependencies was throwing a dependency conflict error (I think since jsonify is listed in both LinkingTo: and Suggests: and only once with a minimum version... and that minimum version requires the GitHub version as specified in Remotes:

I guess before this could be released to CRAN, jsonify, rapidjson, and sfheaders would all need to be on CRAN...

@dcooley
Copy link
Collaborator

dcooley commented Nov 20, 2025

excellent, thanks for those changes. Yes I'll update the other packages hopefully in the next couple of days. Then all should be good to go for CRAN

@dcooley dcooley merged commit 5bea8d2 into SymbolixAU:master Nov 23, 2025
5 checks passed
@ateucher ateucher deleted the issue170-mixed-logical branch November 27, 2025 19:40
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.

logicals in geojson get converted to character 0/1 in geojson_sf()

2 participants