vatsimnetwork / vatspy-data-project

A home for VATSPY Data updates.
Creative Commons Attribution Share Alike 4.0 International
100 stars 136 forks source link

Switching to GeoJSON #389

Closed NelisV closed 2 years ago

NelisV commented 3 years ago

Hey All,

Within the project we've been discussing the possibility to switch to GeoJSON for general maintenance of the data set. To me this seems like a good idea, the firboundaries.dat format has served us well for a long time but better, more standardized formats exist that allow for easier use with GIS tools to edit and standard frameworks to display the data.

My proposal would be to convert the current dataset to GeoJSON at the end of an AIRAC cycle and from that point on edit that GeoJSON file. So when the next AIRAC date comes along, we can release that updated GeoJSON file + use it to generate a firboundaries.dat file for use with VAT-Spy.

I've created a converter so a test firboundaries.dat file can be generated at any time by the people submitting changes, even tho the submit itself would be done in GeoJSON.

I'm interested to hear your opinions on this as users of the dataset, do you use the data in it's .dat format? do you convert it to another data format?

dsolesvik commented 3 years ago

Hi Niels,

I agree with the idea - this seems like a good idea, as it would save time and effort with converting the file when working with QGIS. My main concern is, will github be able to properly read GeoJSON files, just as it is able to do so with .dat right now with proper formatting, so that you can see all changes correctly? Also, are you proposing that we use GeoJSON for the Github specifically, and then convert to .dat for each airac cycle, or are you proposing to change the code within VATSpy itself?

NelisV commented 3 years ago

Github supports geojson, can even draw it on a map which is really nice, other than that it handles it in the same way as .dat files. In the short term VAT-Spy wont be changed, but as long as both files are there, there's always the option down the road.

maiuswong commented 3 years ago

Hi Niels,

Meant to reply to this earlier. SimAware already uses a converted GeoJSON file, so from my point of view this would remove a step and would make automating the process much easier. We'd probably want to wait for the other maps to chime in but I support this move.

meltinglava commented 3 years ago

Had a look through the converter. One thing i found odd was that geometry was defined as a multipolygon instead of a polygon. Any reason for this?

meltinglava commented 3 years ago

Also. I think we should calculate the values we can find in the geojson format to give back to FIRBoundaries.dat like (min/max)(lat/lon) and PointCount. Then we do not need to validate all of these fields. The id field are also redundant we can sort output based on icao (or use an indexed map)(like pythons standard dict)

meltinglava commented 3 years ago

looks like ALOT of the (min/max)_(lat/lon) stated in FIRBoundaries.dat are not up to date with the actual boundaries. There should also be a discussion if all of the boundaries in FIRBoundaries.dat should be explicitly or implicitly closed (aka the first and last point beeing the same). For info geojson states that polygons NEEDS to be explicitly closed. In FIRBoundaries.dat bought implicit and explicit closing happens

NelisV commented 3 years ago

geometry was defined as a multipolygon instead of a polygon.

This allows for geometry to be 'cut out' of other geometry, could be handy in some places and is fully supported by most mapping frameworks.

I think we should calculate the values we can find in the geojson format to give back to FIRBoundaries.dat like (min/max)(lat/lon) and PointCount.

I don't think these values would be necessary, they are as far as i know only used by vatspy, and should be calculated by the application based on the geometry. Renaming the center lat/lon to label position would also be a good step.

The id field are also redundant we can sort output based on icao (or use an indexed map)(like pythons standard dict)

ICAO is currently not unique. Having a unique ID other than the icao enables some handy things with sector <-> station allocation in the future.

For info geojson states that polygons NEEDS to be explicitly closed.

indeed, see point 2 in 3.1.6, the parses already corrects for this.

The parser was just a means to an end to test the data, not meant as the final format. I'll make a more solid proposal somewhere in the coming days and add to this chat.

Thanks all for the comments 👍🏻

meltinglava commented 3 years ago

I have made an almost complete version in rust (from a parser i wrote way back when). If you want to have a a look at it its here

marvk commented 3 years ago

Hey Niels, as we discussed on Discord I think this is a good idea. Standardized parsers exist for JSON/GeoJSON and thus will make the data more accessible to new projects. And speaking as someone building an existing project, switching over could be done with little effort.

The converter should probably be two-way so that the deprecated format can be maintained with little effort before an eventual sunsetting in the future.

meltinglava commented 3 years ago

The converter should probably be two-way so that the deprecated format can be maintained with little effort before an eventual sunsetting in the future.

The converter i made supports conversions bought ways. If you want to test it.

meltinglava commented 3 years ago

These sectors are defined clockwise. All other are counterclockwise. Will be needed to swap directions when switching to geojson

AYPM
BIRD
BIRD-E
BIRD-W
EGTT-W
EIDW
FZZA
GMMM
GMMM
GMMO
HCSM
HTDC
KZAK
KZAK
LCCC-W
NZOH
NZCK
RJTG
SACO
BAIRES
VDPF
VLVT
VANP
VOMX
VOBL
VEBN
VECC
VECC-B
VECC-R
VVHM
WAAF-W
WBGG
WIIF-E
ZYTL-AR03
ZYTL-AR03
PGZU
meltinglava commented 3 years ago

The id field are also redundant we can sort output based on icao (or use an indexed map)(like pythons standard dict)

ICAO is currently not unique. Having a unique ID other than the icao enables some handy things with sector <-> station allocation in the future.

If we save id in geojson and want to insert a sector in between some other sectors, you will have to change all the following ids in the entire file.

vatpacais commented 3 years ago

re my comments in #421 . Happy to change the generation of our VATPAC FIRBoundaries to check for and reverse any clockwise boundaries. I'm confused by the requirement for no duplicate coordinates in a boundary, when the geojson spec requires the first and last point to be the same in a polygon. I can easily remove the last point when generating the boundaries from our source file, but wanted to check first.

meltinglava commented 3 years ago

I'm confused by the requirement for no duplicate coordinates in a boundary, when the geojson spec requires the first and last point to be the same in a polygon.

Think what is wanted is to have no duplicates in FIRBoundaries.dat and only first and last be duplicates in the geo_json file.

vatpacais commented 2 years ago

Before I start reworking the production of our sectors in the new format I wanted to confirm the requirements for the vatSpy FIR format

Is this correct?

Do we also need to produce them in the geojson format or is that handled by the vatspy data project?

NelisV commented 2 years ago

Hey There, happy to elaborate, you can find all the details in the GeoJSON spec.

* No closing end point for polygons

From 3.1.6 "The first and last positions are equivalent, and they MUST contain identical values; their representation SHOULD also be identical."

* Polygons must be in the anticlockwise direction

Also 3.1.6 "A linear ring MUST follow the right-hand rule with respect to the area it bounds, i.e., exterior rings are counterclockwise, and holes are clockwise." But since we do not use polygons with holes in them so far, everything is counterclockwise.

* All shared poly borders must contain the same data points even if an arc

This is not something that is defined in the spec, but is important to create a well performing map on a variety of platforms. The more overlaps in the dataset, the more resources it will take to draw it on a screen. If you have 2 arcs without common points in roughly the same space it will overlap 2 times for every point in the arc. The rule from our perspective will be that gaps and unintended overlaps will not be allowed.

* Polys that cross the anitmeridan must be drawn as two anticlockwise polys with the antimeridan
  as the common boundary

Correct, as per 3.1.9. "A rectangle extending from 40 degrees N, 170 degrees E across the antimeridian to 50 degrees N, 170 degrees W should be cut in two and represented as a MultiPolygon."

Do we also need to produce them in the geojson format or is that handled by the vatspy data project?

We will only use geojson on the repository and at the end of each cycle a firboundaries.dat file will be generated by us. A simple tool to generate a firboundaries.dat for testing purposes will also be made available.

vatpacais commented 2 years ago

Thanks, that helps clarify it for me. Can you please point me to your geojson file so I can paste it in VS to produce a class I can populate and serialise out as json. You know doubt have parameter names for what's in the old FIR header lines in the json file.

I'd also suggest you notify each person authorised to submit PRs to this repo of the new requirements as many like me will only visit the repo when we have changes to submit and all we can see are the .dat files. I was not aware you were fixing our previous version or the move to geojson. Had I been, I could started work on updating our data.

NelisV commented 2 years ago

I've just created a new branch that includes the proposal, please feel free to comment!

edit: As an example, i've added KZAK as an anti-meridian crossing multipolygon as the RFC describes it, the alternative is to keep these as separate multipolygons so each side can have their own label.

vatpacais commented 2 years ago

I've just created a new branch that includes the proposal, please feel free to comment!

Sorry, am I missing something? That branch just contains the .dat files, I didn't see the proposed geojson file.

Happy to support the move to geojson. How will we manage id numbers if we add/delete an FIR? Do you plan to have a parser that takes the final updated geojson and resets the id's to be in sequence?

vatpacais commented 2 years ago

I noticed that in airac 2112 that the GeoJSON NFFF polygons are the reverse of the correct sequence in 2111 FIRBoundaries,dat and are missing the closing coordinate. There might have been an issue when converting from 2111 FIRBoundaries.dat

NelisV commented 2 years ago

It looks like both polygons in NFFF are closed properly (same first and last coordinates). The order was probably changed when i applied a geometry fix from qgis to the entire dataset. Everything is valid and working as expected now tho.