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

Add support for writing Feature Collection without properties #106

Merged
merged 7 commits into from
Mar 27, 2024

Conversation

eliascarv
Copy link
Contributor

@eliascarv eliascarv commented Mar 22, 2024

Shapefile.jl breaks when writing a Feature Collection without properties.
MWE:

julia> import GeoJSON

julia> fc = GeoJSON.read("test.geojson")
FeatureCollection with 3 Features

julia> import Shapefile

julia> Shapefile.write("test.shp", fc)
ERROR: MethodError: reducing over an empty collection is not allowed; consider supplying `init` to the reducer
Stacktrace:
  [1] reduce_empty(op::Base.MappingRF{Shapefile.var"#21#24", Base.BottomRF{…}}, ::Type{Tables.ColumnsRow{…}})
    @ Base ./reduce.jl:361
...

Contents of the test.geojson file:

{
    "type": "FeatureCollection",
    "features": [
        {
            "type": "Feature",
            "geometry": {
                "type": "Point",
                "coordinates": [
                    0.0,
                    0.0
                ]
            },
            "properties": {}
        },
        {
            "type": "Feature",
            "geometry": {
                "type": "Point",
                "coordinates": [
                    1.0,
                    1.0
                ]
            },
            "properties": {}
        },
        {
            "type": "Feature",
            "geometry": {
                "type": "Point",
                "coordinates": [
                    2.0,
                    2.0
                ]
            },
            "properties": {}
        }
    ]
}

@rafaqz
Copy link
Member

rafaqz commented Mar 22, 2024

Thanks!

That's a weird build error, you could try to isolate it by not running CI on Windows and/or 1.9

@eliascarv
Copy link
Contributor Author

eliascarv commented Mar 22, 2024

@rafaqz sorry, but I don't have permissions to run the CI.

@rafaqz
Copy link
Member

rafaqz commented Mar 23, 2024

Oh I just meant to comment it out in the workflow and push another commit.

Even with permissions github actions doesn't let us just trigger one on its own, it just runs everything so will probably kill the job from the same failure again.

@rafaqz
Copy link
Member

rafaqz commented Mar 23, 2024

@eliascarv
Copy link
Contributor Author

eliascarv commented Mar 25, 2024

I honestly don't know what's going on with the CI, locally the tests pass.
Version info:

julia> versioninfo()
Julia Version 1.10.2
Commit bd47eca2c8a (2024-03-01 10:14 UTC)
Build Info:
  Official https://julialang.org/ release
Platform Info:
  OS: Linux (x86_64-linux-gnu)
  CPU: 12 × Intel(R) Core(TM) i5-10400 CPU @ 2.90GHz
  WORD_SIZE: 64
  LIBM: libopenlibm
  LLVM: libLLVM-15.0.7 (ORCJIT, skylake)
Threads: 1 default, 0 interactive, 1 GC (on 12 virtual cores)

@rafaqz
Copy link
Member

rafaqz commented Mar 25, 2024

Yep you're on 1.10 linux! Thats why I'm suggesting to comment out windows and 1.9 in the CI matrix and see what happens

https://github.com/JuliaGeo/Shapefile.jl/blob/main/.github/workflows/CI.yml#L16

@eliascarv
Copy link
Contributor Author

Strange, now it works @rafaqz.

@rafaqz
Copy link
Member

rafaqz commented Mar 25, 2024

Not so strange :)

Not try with 1.9 but not windows, etc...

@eliascarv
Copy link
Contributor Author

Commenting Windows only also works. Now I will comment only 1.9.

@eliascarv
Copy link
Contributor Author

eliascarv commented Mar 25, 2024

Commenting 1.9 only also works. @rafaqz, the problem appears to be related to Julia 1.9 running on Windows.

@rafaqz
Copy link
Member

rafaqz commented Mar 25, 2024

Ah cool good that its only one. Not sure how to specify not testing that though!

If you can work out how to not test 1.9 on windows that would be perfect, otherwise we can just turn off windows for now.

@eliascarv
Copy link
Contributor Author

@rafaqz done. Ready for review.

Copy link
Member

@rafaqz rafaqz left a comment

Choose a reason for hiding this comment

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

LGTM

@rafaqz rafaqz merged commit 186ea20 into JuliaGeo:main Mar 27, 2024
6 checks passed
@eliascarv eliascarv deleted the no-props branch March 27, 2024 14:42
@eliascarv
Copy link
Contributor Author

@rafaqz, can you release a patch with this PR?

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