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

Switch on writing of VRTs by default #956

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open

Switch on writing of VRTs by default #956

wants to merge 2 commits into from

Conversation

olsen232
Copy link
Collaborator

@olsen232 olsen232 commented Dec 5, 2023

... during working copy checkout of raster datasets.
Adds a utility method to check if an environment variable is turned off since bool(os.environ.get(key)) won't work anymore.

Checklist:

  • Have you reviewed your own change?
  • Have you included test(s)?
  • Have you updated the changelog?

@@ -58,4 +58,4 @@ For technical reasons, raster tiles are stored within the repository using `Git

VRT files
~~~~~~~~~
Setting an environment variable ``KART_RASTER_VRTS=1`` when creating the Kart working copy or checking out a commit will cause Kart to create a `VRT <vrt_>`_ (Virtual Raster) file for each raster dataset. This single file comprises a mosaic of all the tiles in the working copy that belong to that dataset, so that if you load this file in your tile-editing software, you have effectively loaded the entire dataset. The individual tiles are referenced by the VRT, rather than the data from each tile being duplicated in the VRT. Creation of VRTs is still experimental but should become the default in a future version of Kart.
When creating the working copy or checking out a commit, Kart will create a `VRT <vrt_>`_ (Virtual Raster) file for each raster dataset. This single file comprises a mosaic of all the tiles in the working copy that belong to that dataset, so that if you load this file in your tile-editing software, you have effectively loaded the entire dataset. The individual tiles are referenced by the VRT, rather than the data from each tile being duplicated in the VRT. To turn off the automatic creation of VRTs, set environment variable ``KART_RASTER_VRTS`` to ``0``.
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
When creating the working copy or checking out a commit, Kart will create a `VRT <vrt_>`_ (Virtual Raster) file for each raster dataset. This single file comprises a mosaic of all the tiles in the working copy that belong to that dataset, so that if you load this file in your tile-editing software, you have effectively loaded the entire dataset. The individual tiles are referenced by the VRT, rather than the data from each tile being duplicated in the VRT. To turn off the automatic creation of VRTs, set environment variable ``KART_RASTER_VRTS`` to ``0``.
When creating the working copy or checking out a commit, Kart will create a `VRT <vrt_>`_ (Virtual Raster) file for each raster dataset. This single file comprises a mosaic of all the tiles in the working copy that belong to that dataset, so that if you load this file in your GIS software, you have effectively loaded the entire dataset as one layer. To turn off the automatic creation of VRTs, set environment variable ``KART_RASTER_VRTS`` to ``0``.

not sure what that sentence means, doesn't seem to add much

@hamishcampbell
Copy link
Member

We're undertaking some software testing to make sure this behaviour won't have unintended side-effects in some commonly used software packages.

during working copy checkout of raster datasets.
Adds a utility method to check if an environment variable is turned off
since bool(os.environ.get(key)) won't work anymore.
@@ -32,4 +32,9 @@ def write_vrt_for_directory(directory_path):
del vrt

vrt_text = vrt_path.read_text()
vrt_text = vrt_text.replace(
"</VRTDataset>",
' <OverviewList resampling="average">2 4 8</OverviewList>\n</VRTDataset>',
Copy link
Member

Choose a reason for hiding this comment

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

How do we know that 2,4,8 is the right list here?

AFAIK we're not storing overview info in the schema, so we should really be sampling the tiles.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Yeah... we don't
I pushed this last night so we can better test the effect of having this element. But I don't really understand this element: need to figure out what it should contain when and why.
I should have put a TODO, I won't merge this until I at least know some more

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.

4 participants