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

feat!: Auto-fill GSD unit TDE-1211 #1089

Merged
merged 5 commits into from
Sep 26, 2024
Merged

feat!: Auto-fill GSD unit TDE-1211 #1089

merged 5 commits into from
Sep 26, 2024

Conversation

l0b0
Copy link
Contributor

@l0b0 l0b0 commented Sep 23, 2024

Motivation

This is always in metres, so we shouldn't ask the end user to fill it in.

Modifications

Change GSD parameters to be decimal numbers, rather than strings

Verification

pytest

This is always in metres, so we shouldn't ask the end user to fill it
in.
@l0b0 l0b0 marked this pull request as ready for review September 23, 2024 03:00
@l0b0 l0b0 requested a review from a team as a code owner September 23, 2024 03:00
@blacha
Copy link
Member

blacha commented Sep 23, 2024

this is a breaking change, can it be tagged as breaking and explain the migration path is it just changing "1m" to "1" in all the CLIs?

Could this change be done in a backwards compatible way?

side note, all the e2e tests have highly uncommon GSDs 60m ? could a few of those be made more realistic, urban imagery is often in the 0.05m, 0.0125m or 0.075m sizes.

@l0b0 l0b0 changed the title feat: Auto-fill GSD unit TDE-1211 feat!: Auto-fill GSD unit TDE-1211 Sep 23, 2024
@l0b0
Copy link
Contributor Author

l0b0 commented Sep 23, 2024

this is a breaking change, can it be tagged as breaking […]

Done.

and explain the migration path is it just changing "1m" to "1" in all the CLIs?

Yep, where does such migration documentation go?

Could this change be done in a backwards compatible way?

Of course. But how difficult would it be to move our entire customer base to use the new syntax?

side note, all the e2e tests have highly uncommon GSDs 60m ? could a few of those be made more realistic, urban imagery is often in the 0.05m, 0.0125m or 0.075m sizes.

Do you want to file an issue for this?

@amfage amfage removed the request for review from MDavidson17 September 23, 2024 20:37
l0b0 added a commit to linz/topo-workflows that referenced this pull request Sep 24, 2024
dependabot bot and others added 4 commits September 26, 2024 12:40
Bumps [pytest](https://github.com/pytest-dev/pytest) from 8.3.2 to
8.3.3.
<details>
<summary>Release notes</summary>
<p><em>Sourced from <a
href="https://github.com/pytest-dev/pytest/releases">pytest's
releases</a>.</em></p>
<blockquote>
<h2>8.3.3</h2>
<h1>pytest 8.3.3 (2024-09-09)</h1>
<h2>Bug fixes</h2>
<ul>
<li>
<p><a
href="https://github.com/pytest-dev/pytest/issues/12446">#12446</a>:
Avoid calling <code>@Property</code> (and other instance descriptors)
during fixture discovery -- by <code>asottile</code>{.interpreted-text
role=&quot;user&quot;}</p>
</li>
<li>
<p><a
href="https://github.com/pytest-dev/pytest/issues/12659">#12659</a>:
Fixed the issue of not displaying assertion failure differences when
using the parameter <code>--import-mode=importlib</code> in
pytest&gt;=8.1.</p>
</li>
<li>
<p><a
href="https://github.com/pytest-dev/pytest/issues/12667">#12667</a>:
Fixed a regression where type change in
[ExceptionInfo.errisinstance]{.title-ref} caused [mypy]{.title-ref} to
fail.</p>
</li>
<li>
<p><a
href="https://github.com/pytest-dev/pytest/issues/12744">#12744</a>:
Fixed typing compatibility with Python 3.9 or less -- replaced
[typing.Self]{.title-ref} with [typing_extensions.Self]{.title-ref} --
by <code>Avasam</code>{.interpreted-text role=&quot;user&quot;}</p>
</li>
<li>
<p><a
href="https://github.com/pytest-dev/pytest/issues/12745">#12745</a>:
Fixed an issue with backslashes being incorrectly converted in nodeid
paths on Windows, ensuring consistent path handling across
environments.</p>
</li>
<li>
<p><a
href="https://github.com/pytest-dev/pytest/issues/6682">#6682</a>:
Fixed bug where the verbosity levels where not being respected when
printing the &quot;msg&quot; part of failed assertion (as in
<code>assert condition, msg</code>).</p>
</li>
<li>
<p><a
href="https://github.com/pytest-dev/pytest/issues/9422">#9422</a>:
Fix bug where disabling the terminal plugin via <code>-p
no:terminal</code> would cause crashes related to missing the
<code>verbose</code> option.</p>
<p>-- by <code>GTowers1</code>{.interpreted-text
role=&quot;user&quot;}</p>
</li>
</ul>
<h2>Improved documentation</h2>
<ul>
<li><a
href="https://github.com/pytest-dev/pytest/issues/12663">#12663</a>:
Clarify that the [pytest_deselected]{.title-ref} hook should be called
from [pytest_collection_modifyitems]{.title-ref} hook implementations
when items are deselected.</li>
<li><a
href="https://github.com/pytest-dev/pytest/issues/12678">#12678</a>:
Remove erroneous quotes from [tmp_path_retention_policy]{.title-ref}
example in docs.</li>
</ul>
<h2>Miscellaneous internal changes</h2>
<ul>
<li><a
href="https://github.com/pytest-dev/pytest/issues/12769">#12769</a>:
Fix typos discovered by codespell and add codespell to pre-commit
hooks.</li>
</ul>
</blockquote>
</details>
<details>
<summary>Commits</summary>
<ul>
<li><a
href="https://github.com/pytest-dev/pytest/commit/d0f136fe64f9374f18a04562305b178fb380d1ec"><code>d0f136f</code></a>
build(deps): Bump pypa/gh-action-pypi-publish from 1.10.0 to 1.10.1 (<a
href="https://github.com/pytest-dev/pytest/issues/12790">#12790</a>)</li>
<li><a
href="https://github.com/pytest-dev/pytest/commit/972f307c7861ae498e705d3d12e003fa4b035ac0"><code>972f307</code></a>
Prepare release version 8.3.3</li>
<li><a
href="https://github.com/pytest-dev/pytest/commit/0dabdcfe4de99147a07bd577804b60818ea25bc4"><code>0dabdcf</code></a>
Include co-authors in release announcement (<a
href="https://github.com/pytest-dev/pytest/issues/12795">#12795</a>)
(<a
href="https://github.com/pytest-dev/pytest/issues/12797">#12797</a>)</li>
<li><a
href="https://github.com/pytest-dev/pytest/commit/a9910a413a691e1b216e2235a9cbec0921117702"><code>a9910a4</code></a>
Do not discover properties when iterating fixtures (<a
href="https://github.com/pytest-dev/pytest/issues/12781">#12781</a>)
(<a
href="https://github.com/pytest-dev/pytest/issues/12788">#12788</a>)</li>
<li><a
href="https://github.com/pytest-dev/pytest/commit/0f10b6b0d8138d3539de75cb7b2e33167b6fc882"><code>0f10b6b</code></a>
Fix issue with slashes being turned into backslashes on Windows (<a
href="https://github.com/pytest-dev/pytest/issues/12760">#12760</a>)
(<a
href="https://github.com/pytest-dev/pytest/issues/12">#12</a>...</li>
<li><a
href="https://github.com/pytest-dev/pytest/commit/300d13d2231db85186729c2091ea33480cb39c1a"><code>300d13d</code></a>
Merge pull request <a
href="https://github.com/pytest-dev/pytest/issues/12785">#12785</a>
from pytest-dev/patchback/backports/8.3.x/57cccf7f4...</li>
<li><a
href="https://github.com/pytest-dev/pytest/commit/e5d32c73abcf4fa1362b15aaf660074de8f710d4"><code>e5d32c7</code></a>
Merge pull request <a
href="https://github.com/pytest-dev/pytest/issues/12784">#12784</a>
from svenevs/fix/docs-example-parametrize-minor-typo</li>
<li><a
href="https://github.com/pytest-dev/pytest/commit/bc913d194ec009699194b016ca619d5ae7f22c91"><code>bc913d1</code></a>
Streamline checks for verbose option (<a
href="https://github.com/pytest-dev/pytest/issues/12706">#12706</a>)
(<a
href="https://github.com/pytest-dev/pytest/issues/12778">#12778</a>)</li>
<li><a
href="https://github.com/pytest-dev/pytest/commit/01cfcc9f2dda817b25511772593012fd93e092d0"><code>01cfcc9</code></a>
Fix typos and introduce codespell pre-commit hook (<a
href="https://github.com/pytest-dev/pytest/issues/12769">#12769</a>)
(<a
href="https://github.com/pytest-dev/pytest/issues/12774">#12774</a>)</li>
<li><a
href="https://github.com/pytest-dev/pytest/commit/4873394d53635ef62d1915d23972ed4281a784eb"><code>4873394</code></a>
doc: Remove past training (<a
href="https://github.com/pytest-dev/pytest/issues/12772">#12772</a>)
(<a
href="https://github.com/pytest-dev/pytest/issues/12773">#12773</a>)</li>
<li>Additional commits viewable in <a
href="https://github.com/pytest-dev/pytest/compare/8.3.2...8.3.3">compare
view</a></li>
</ul>
</details>
<br />


[![Dependabot compatibility
score](https://dependabot-badges.githubapp.com/badges/compatibility_score?dependency-name=pytest&package-manager=pip&previous-version=8.3.2&new-version=8.3.3)](https://docs.github.com/en/github/managing-security-vulnerabilities/about-dependabot-security-updates#about-compatibility-scores)

Dependabot will resolve any conflicts with this PR as long as you don't
alter it yourself. You can also trigger a rebase manually by commenting
`@dependabot rebase`.

[//]: # (dependabot-automerge-start)
[//]: # (dependabot-automerge-end)

---

<details>
<summary>Dependabot commands and options</summary>
<br />

You can trigger Dependabot actions by commenting on this PR:
- `@dependabot rebase` will rebase this PR
- `@dependabot recreate` will recreate this PR, overwriting any edits
that have been made to it
- `@dependabot merge` will merge this PR after your CI passes on it
- `@dependabot squash and merge` will squash and merge this PR after
your CI passes on it
- `@dependabot cancel merge` will cancel a previously requested merge
and block automerging
- `@dependabot reopen` will reopen this PR if it is closed
- `@dependabot close` will close this PR and stop Dependabot recreating
it. You can achieve the same result by closing it manually
- `@dependabot show <dependency name> ignore conditions` will show all
of the ignore conditions of the specified dependency
- `@dependabot ignore this major version` will close this PR and stop
Dependabot creating any more for this major version (unless you reopen
the PR or upgrade to it yourself)
- `@dependabot ignore this minor version` will close this PR and stop
Dependabot creating any more for this minor version (unless you reopen
the PR or upgrade to it yourself)
- `@dependabot ignore this dependency` will close this PR and stop
Dependabot creating any more for this dependency (unless you reopen the
PR or upgrade to it yourself)


</details>

Signed-off-by: dependabot[bot] <[email protected]>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
### Motivation


https://github.com/radiantearth/stac-spec/blob/master/commons/links.md#stac-media-types

### Modifications

Change item links from media type `application/json` to
`application/geo+json`.

### Verification

`pytest`
🤖 I have created a release *beep* *boop*
---


##
[4.11.2](v4.11.1...v4.11.2)
(2024-09-24)


### Bug Fixes

* Use GeoJSON media type for item links TDE-1254
([#1086](#1086))
([57dd0a8](57dd0a8))

---
This PR was generated with [Release
Please](https://github.com/googleapis/release-please). See
[documentation](https://github.com/googleapis/release-please#release-please).

Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
For backwards compatibility.

Also treat it as deprecated.
@l0b0 l0b0 enabled auto-merge September 26, 2024 01:34
@l0b0 l0b0 added this pull request to the merge queue Sep 26, 2024
@@ -31,23 +31,23 @@ jobs:

- name: End to end test - Aerial Imagery
run: |
docker run -v "${{ runner.temp }}:/tmp/" topo-imagery python3 standardise_validate.py --from-file ./tests/data/aerial.json --preset webp --target-epsg 2193 --source-epsg 2193 --target /tmp/ --collection-id 123 --start-datetime 2023-01-01 --end-datetime 2023-01-01 --gsd 10m --create-footprints=true
docker run -v "${{ runner.temp }}:/tmp/" topo-imagery python3 standardise_validate.py --from-file ./tests/data/aerial.json --preset webp --target-epsg 2193 --source-epsg 2193 --target /tmp/ --collection-id 123 --start-datetime 2023-01-01 --end-datetime 2023-01-01 --gsd 10 --create-footprints=true
Copy link
Collaborator

Choose a reason for hiding this comment

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

Might be good to have one of the test keeping the m?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I did:

Merged via the queue into master with commit df257c9 Sep 26, 2024
2 checks passed
@l0b0 l0b0 deleted the feat/auto-fill-gsd-unit branch September 26, 2024 21:46
@github-actions github-actions bot mentioned this pull request Sep 26, 2024
l0b0 added a commit that referenced this pull request Sep 27, 2024
### Motivation

This is always in metres, so we shouldn't ask the end user to fill it
in.

### Modifications

Change GSD parameters to be decimal numbers, rather than strings

### Verification

`pytest`

---------

Signed-off-by: dependabot[bot] <[email protected]>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
github-merge-queue bot pushed a commit to linz/topo-workflows that referenced this pull request Sep 29, 2024
#### Motivation

linz/topo-imagery#1089

#### Checklist

- [ ] Tests updated (N/A)
- [x] Docs updated
- [x] Issue linked in Title
github-merge-queue bot pushed a commit that referenced this pull request Sep 29, 2024
🤖 I have created a release *beep* *boop*
---


##
[5.0.0](v4.11.2...v5.0.0)
(2024-09-29)


### ⚠ BREAKING CHANGES

* dates from item TDE-1258
([#1099](#1099))
* Auto-fill GSD unit TDE-1211
([#1089](#1089))

### Features

* Add topo-imagery version information to STAC TDE-1265
([#1080](#1080))
([2039606](2039606))
* Auto-fill GSD unit TDE-1211
([#1089](#1089))
([df257c9](df257c9))
* dates from item TDE-1258
([#1099](#1099))
([8aabe64](8aabe64))

---
This PR was generated with [Release
Please](https://github.com/googleapis/release-please). See
[documentation](https://github.com/googleapis/release-please#release-please).

Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

3 participants