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
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ _When adding new entries to the changelog, please include issue/PR numbers where
- Prevented committing local changes to linked datasets. [#953](https://github.com/koordinates/kart/pull/953)
- Fixes a bug where even datasets marked as `--no-checkout` are checked out when the working copy is first created. [#954](https://github.com/koordinates/kart/pull/954)
- Fixes a bug where minor changes to auxiliary metadata (.aux.xml) files that Kart is supposed to ignore were preventing branch changes. [#957](https://github.com/koordinates/kart/pull/957)
- Enables VRT creation by default during raster dataset checkout. [#956](https://github.com/koordinates/kart/pull/956)

## 0.15.0

Expand Down
2 changes: 1 addition & 1 deletion docs/pages/raster_datasets.rst
Original file line number Diff line number Diff line change
Expand Up @@ -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

16 changes: 16 additions & 0 deletions kart/cli_util.py
Original file line number Diff line number Diff line change
Expand Up @@ -424,3 +424,19 @@ def forward_context_to_command(ctx, command):
subctx = command.make_context(command.name, ctx.unparsed_args)
subctx.obj = ctx.obj
subctx.forward(command)


def get_bool_from_env(env_var_name, default=False):
"""
Checks what the user has requested in terms of environment variables in the form:
SOME_FEATURE=YES or OTHER_FEATURE=0
- 0, N, NO, FALSE and OFF are falsey (case insensitive)
- every other string is truthy
- if the variable is unset, the default is returned.
"""
env_var_val = os.environ.get(env_var_name)
if env_var_val is None:
return default
if env_var_val.upper() in ("0", "N", "NO", "FALSE", "OFF"):
return False
return True
5 changes: 5 additions & 0 deletions kart/raster/mosaic_util.py
Original file line number Diff line number Diff line change
Expand Up @@ -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

)

vrt_path.write_text(KART_VRT_HEADING + vrt_text)
4 changes: 2 additions & 2 deletions kart/raster/v1.py
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import functools
import os
import re

from kart.cli_util import get_bool_from_env
from kart.core import all_blobs_in_tree
from kart.diff_structs import DeltaDiff, Delta
from kart.key_filters import FeatureKeyFilter
Expand Down Expand Up @@ -80,7 +80,7 @@ def convert_tile_to_format(cls, source_path, dest_path, target_format):
def write_mosaic_for_directory(cls, directory_path):
from kart.raster.mosaic_util import write_vrt_for_directory

if os.environ.get("KART_RASTER_VRTS"):
if get_bool_from_env("KART_RASTER_VRTS", default=True):
write_vrt_for_directory(directory_path)

@classmethod
Expand Down
4 changes: 2 additions & 2 deletions tests/raster/test_workingcopy.py
Original file line number Diff line number Diff line change
Expand Up @@ -731,8 +731,6 @@ def test_working_copy_conflicting_extension(cli_runner, data_archive):


def test_working_copy_vrt(cli_runner, data_archive, monkeypatch):
monkeypatch.setenv("KART_RASTER_VRTS", "1")

with data_archive("raster/elevation.tgz") as repo_path:
vrt_path = repo_path / "elevation" / "elevation.vrt"

Expand All @@ -758,6 +756,8 @@ def test_working_copy_vrt(cli_runner, data_archive, monkeypatch):


def test_working_copy_vrt_disabled(cli_runner, data_archive, monkeypatch):
monkeypatch.setenv("KART_RASTER_VRTS", "0")

with data_archive("raster/elevation.tgz") as repo_path:
shutil.rmtree(repo_path / "elevation")

Expand Down