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.

Sign up for GitHub

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

Jump to bottom

raster: Rename band references to semantic labels #1928

Merged
merged 11 commits into from
Nov 14, 2021
Merged

raster: Rename band references to semantic labels #1928

merged 11 commits into from
Nov 14, 2021

Conversation

marisn
Copy link
Contributor

@marisn marisn commented Oct 5, 2021

See discussion in #1868

@marisn marisn mentioned this pull request Oct 12, 2021
Closed
@marisn marisn marked this pull request as ready for review October 22, 2021 10:17
@marisn
Copy link
Contributor Author

marisn commented Oct 22, 2021

The OSGeo4W test fail doesn't seem to be related to this PR.

@marisn
Copy link
Contributor Author

marisn commented Oct 29, 2021

Ping @landam @wenzeslaus May I merge this?

@marisn marisn added this to the 8.0.0 milestone Oct 29, 2021
@marisn marisn added the blocker Blocking a release label Oct 29, 2021
@marisn marisn linked an issue Oct 29, 2021 that may be closed by this pull request
[Bug] Band references need a better name and definition #1868
Closed
wenzeslaus
wenzeslaus approved these changes Nov 12, 2021
View reviewed changes
Copy link
Member

@wenzeslaus wenzeslaus left a comment

Choose a reason for hiding this comment

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

Great.

I'm just not sure if you meant to change the temporal modules or not. For example, t.rast.list.html still talks about band references, or is that on purpose?

@marisn
Copy link
Contributor Author

marisn commented Nov 12, 2021

I'm just not sure if you meant to change the temporal modules or not. For example, t.rast.list.html still talks about band references, or is that on purpose?

Hard to tell if I messed up during rebase (again) or just skipped. Went over and replaced even more locations of band references with semantic label.
Now should only i.bands and g.bands remain. What should be the new name for each of them?

@wenzeslaus
Copy link
Member

Now should only i.bands and g.bands remain. What should be the new name for each of them?

During the PSC meeting today we talked about options such as my suggestion:

  • i.bands -> r.semantic.label (Assigns semantic label to a raster)
  • g.bands -> i.band.library (Register band metadata for a semantic label)

...but there was no final decision made.

wenzeslaus
wenzeslaus approved these changes Nov 12, 2021
View reviewed changes
temporal/t.register/t.register.html Outdated Show resolved Hide resolved
@marisn
Copy link
Contributor Author

marisn commented Nov 14, 2021

During the PSC meeting today we talked about options such as my suggestion:
...but there was no final decision made.

Then lets do it in a separate step. I'll merge this one and then we can handle renaming (or deactivating compilation) of modules as a separate PR.

@marisn marisn merged commit 0e43bf3 into OSGeo:main Nov 14, 2021
@marisn marisn deleted the bandref_rename branch November 14, 2021 16:29
@landam
Copy link
Member

landam commented Nov 14, 2021

Sorry for being late with review. Let's backport to releasebranch_8_0.

@neteler
Copy link
Member

neteler commented Nov 14, 2021

BTW: Shall we have separate G7 and G8 backport labels?

@landam
Copy link
Member

landam commented Nov 14, 2021

I think that it's not needed. Milestone is set to 8.0.0, so only backport to releasebranch_8_0 is relevant.

@marisn marisn mentioned this pull request Nov 22, 2021
Merged
neteler pushed a commit that referenced this pull request Nov 22, 2021
@marisn @neteler
Rename band references to semantic labels (#1928)
f0f6d65 
Rename band references to semantic labels (fixes #1868)
@neteler
Copy link
Member

neteler commented Nov 22, 2021

I think that it's not needed. Milestone is set to 8.0.0, so only backport to releasebranch_8_0 is relevant.

Backport to releasebranch_8_0 done by cherry-picking.

marisn added a commit that referenced this pull request Nov 23, 2021
@marisn
Rename g.bads to i.band.library and i.band to r.semantic.label (#1983)
eb84192 
According to a proposal by PSC:
https://trac.osgeo.org/grass/wiki/PSC/Minutes/PSC_Meeting_20211112

Related to #1868 and #1928
neteler pushed a commit that referenced this pull request Nov 23, 2021
@marisn @neteler
Rename g.bads to i.band.library and i.band to r.semantic.label (#1983)
8d1b5ba 
According to a proposal by PSC:
https://trac.osgeo.org/grass/wiki/PSC/Minutes/PSC_Meeting_20211112

Related to #1868 and #1928
@neteler neteler mentioned this pull request Feb 23, 2022
Closed
@wenzeslaus wenzeslaus changed the title Rename band references to semantic labels raster: Rename band references to semantic labels Apr 27, 2022
ninsbl pushed a commit to ninsbl/grass that referenced this pull request Oct 26, 2022
@marisn @ninsbl
Rename band references to semantic labels (OSGeo#1928)
b98a46d 
Rename band references to semantic labels (fixes OSGeo#1868)
ninsbl pushed a commit to ninsbl/grass that referenced this pull request Oct 26, 2022
@marisn @ninsbl
Rename g.bads to i.band.library and i.band to r.semantic.label (OSGeo…
b454fc9 
…#1983)

According to a proposal by PSC:
https://trac.osgeo.org/grass/wiki/PSC/Minutes/PSC_Meeting_20211112

Related to OSGeo#1868 and OSGeo#1928
ninsbl pushed a commit to ninsbl/grass that referenced this pull request Feb 17, 2023
@marisn @ninsbl
Rename band references to semantic labels (OSGeo#1928)
f1b5eea 
Rename band references to semantic labels (fixes OSGeo#1868)
ninsbl pushed a commit to ninsbl/grass that referenced this pull request Feb 17, 2023
@marisn @ninsbl
Rename g.bads to i.band.library and i.band to r.semantic.label (OSGeo…
c415b29 
…#1983)

According to a proposal by PSC:
https://trac.osgeo.org/grass/wiki/PSC/Minutes/PSC_Meeting_20211112

Related to OSGeo#1868 and OSGeo#1928
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@wenzeslaus wenzeslaus wenzeslaus approved these changes

@landam landam Awaiting requested review from landam

Assignees
No one assigned
Labels
blocker Blocking a release
Projects
None yet
Milestone
8.0.0
Development

Successfully merging this pull request may close these issues.

[Bug] Band references need a better name and definition
4 participants
@marisn @wenzeslaus @landam @neteler