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

Geojson api and gui #142

Merged
merged 10 commits into from
Mar 8, 2024
Merged

Geojson api and gui #142

merged 10 commits into from
Mar 8, 2024

Conversation

ximenesuk
Copy link
Collaborator

@ximenesuk ximenesuk commented Mar 7, 2024

This PR adds the option for GeoJSON to be included in the JSON validation response data if possible. Two fields have been added to the validation response data: geojson and geojson_error.

  • If the return_geomentry API flag is False or None the fields will be { } and None respectively.
  • If the return_geomentry API flag is True and geometry data was parsed geojson will contain the GeoJSON data and geojson_error will be None
  • If the return_geomentry API flag is True and geometry data failed to be parsed geojson will be { } and geojson_error will be contain a reason that will correspond to a validation error.

The tests should pass. The GUI should be tested with a range of files, test_borehole_map.py contains a useful set or good and bad files.

The PR also fully implements the sorting strategies. All four return whatever the upstream library returned. For some files each option should return a different sheet ordering, "Ashfield Area C Development.ags" illustrates this. The tests only confirm default and alphabetical until the other orders are confirmed.

Finally, the Summary from the AGS checker is added to the data as a summary field containing a list of summary items. This list mostly duplicates the additional_metadata resulting from a BGS check. The schema tests have been updated but there are no explicit tests for summary content in the JSON fixtures. I'll leave this as a discussion point for @volcan01010 and @KoalaGeo!

This PR supersedes #140

@ximenesuk ximenesuk marked this pull request as draft March 7, 2024 16:20
@KoalaGeo
Copy link
Collaborator

KoalaGeo commented Mar 7, 2024

AGS Library Summary and BGS additional metadata duplicate 4 of 5 attributes.

image

AGS Checks should use AGS Summary
BGS Checks should use BGS additional Metadata
AGS & BGS Checks should use BGS additional Metadata

@KoalaGeo
Copy link
Collaborator

KoalaGeo commented Mar 7, 2024

Map should return at end of Validation Results pop up

Use the same topographic & imagery basemaps as the data discovery maps. If can add th OS 50k and 25k at higher resolutions that'd be great.

Map should have link to "Download GeoJSON" which strips the geojson from the validation response and provides as download to the end user.

@ximenesuk
Copy link
Collaborator Author

ximenesuk commented Mar 7, 2024 via email

@KoalaGeo
Copy link
Collaborator

KoalaGeo commented Mar 7, 2024

The AGS `summary` data and BGS `additional_metadata` fields contain
overlapping information, with the BGS response including additional
fields.  This commit avoids repetition by ensuring that if BGS data
are available, only they are included in the response.
@volcan01010 volcan01010 marked this pull request as ready for review March 8, 2024 17:41
@volcan01010
Copy link
Collaborator

I have addressed the summary issue. All the rest looks good, with the JSON appearing in the responses.

@volcan01010 volcan01010 merged commit fe4cebf into main Mar 8, 2024
1 check passed
@volcan01010 volcan01010 deleted the geojson-api branch March 8, 2024 17:43
@KoalaGeo
Copy link
Collaborator

KoalaGeo commented Mar 8, 2024

I have addressed the summary issue. All the rest looks good, with the JSON appearing in the responses.

Can we not make the IF AGS AND/OR BGS make it so the AGS summary responses appear under additional metadata and the BGS attributes then no GUI changes would be required?

bgs_all_groups == summary:0
bgs_file == summary:3
bgs_dict == summary:2
bgs_loca_rows == summary:1
bgs_projects == NULL

@volcan01010
Copy link
Collaborator

Yes, OK. I'll look at that now.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants