Fix invalid WKT empty geometries

[Oreilles]

Currently, empty geometries would be writter in WKT as:

LINESTRING()
POLYGON()
GEOMETRYCOLLECTION()

which isn't valid WKT.

This PR addresses this issue and output instead:

LINESTRING EMPTY
POLYGON EMPTY
GEOMETRYCOLLECTION EMPTY

[Oreilles]

We have a problem with the csv BufferingWktWriter... since it instanciate a new writer for every processor function call, the state isn't persisted and the implementation cannot work.

If we had access to the geometry size in processor functions polygon_end etc., then we wouldn't have to manage the geometry_sizes stack 🤔

[michaelkirk]

We have a problem with the csv BufferingWktWriter... since it instanciate a new writer for every processor function call, the state isn't persisted and the implementation cannot work.

I've just opened https://github.com/Oreilles/geozero/pull/3 against your own branch that I think addresses this (and simplifies my old crappy code from yesteryear!). Take a look and merge into your branch if you think it makes sense.

[michaelkirk]

If we had access to the geometry size in processor functions polygon_end etc., then we wouldn't have to manage the geometry_sizes stack 🤔

I'm also open to this approach (or we could do both) but it seems like a bigger breaking change, so my preference at this point would be to avoid it.

[michaelkirk]

bors r+

[bors[bot]]

Configuration problem: bors.toml: not found

[nyurik]

i thought there were some discussions in rust itself to stop using bors because github now covers most such usecases?

[michaelkirk]

Yes, there were some of us trying to figure out GH merge queues a couple weeks ago. If you look at the proj crate’s recent commits for example, you’ll see some of my misadventures. As far as I know we don’t have any working examples on georust where GH’s merge queues function like bors: i.e. I trigger the process, then the tests run against an up-to-date branch and only get merged into main once they pass.It seems so basic, but i kept finding that the branches would merge immediately, regardless of if the tests pass.I’m sure it’s possible, I just didn’t personally dedicate the time to figuring it out. If you tell me what series of buttons to press I can press them.

[michaelkirk]

I just realized that this project does not, and never did, use bors. 😅

[nyurik]

@michaelkirk in Martin, we have branch protection rule for main:

• Require a pull request before merging
• Require approvals (1)
• Require status checks to pass before merging (with most of the key CI jobs listed as required)

I think if you also enable Require branches to be up to date before merging, you will achieve what you want: a way to +1 something, plus a way to say "squash once CI passes" - this way it will not be squashed unless everything is good, and it will not pass if another PR merges ahead of it. Granted that it will fail if another PR merges first though.

[michaelkirk]

I got it working! Ultimately I think the issue was something else, but thanks for the sanity check @nyurik.