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

i.signatures: Add imagery classifier signature management module #3008

Merged
merged 17 commits into from
Oct 22, 2023

Conversation

marisn
Copy link
Contributor

@marisn marisn commented Jun 4, 2023

For the first time of GRASS history this PR tries to introduce a tool capable of managing signature files.

@marisn marisn added this to the 8.4.0 milestone Jun 4, 2023
@marisn marisn added the C Related code is in C label Jun 4, 2023
@marisn marisn marked this pull request as ready for review June 5, 2023 09:53
@marisn marisn requested a review from wenzeslaus June 5, 2023 09:54
@marisn
Copy link
Contributor Author

marisn commented Jun 5, 2023

One thing to discuss: how should "type" values be named? At the moment they follow internal naming of signature files (i.gensig -> "sig"; i.gensigset -> "sigset") but it might be hard to get it right for users. An alternative could be naming after consumer ("maxlik" and "smap").

Other issue is that the GUI at the moment can not handle signature file name selection as it requires linking "type" with "copy", "rename" and "remove" to list signatures of selected type.

@veroandreo
Copy link
Contributor

One thing to discuss: how should "type" values be named? At the moment they follow internal naming of signature files (i.gensig -> "sig"; i.gensigset -> "sigset") but it might be hard to get it right for users. An alternative could be naming after consumer ("maxlik" and "smap").

I'd prefer to name them as the module that generates them, i.e., i.gensig -> "sig"; i.gensigset -> "sigset", seems more consistent to me.

@wenzeslaus
Copy link
Member

The sig and sigset are indeed confusing, but the producer modules are named that way as well as the internal files, so using sig and sigset is hopefully less confusion overall. Perhaps the consumer modules need to be really good at documenting what they need.

@marisn
Copy link
Contributor Author

marisn commented Jun 5, 2023

Perhaps the consumer modules need to be really good at documenting what they need.

Consumer module documentation lists the correct workflow. Also the parser code will not let to pass wrong signature file. This is only an issue for this module as it allows to operate on all of them.

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.

I'm not sure if this should be one tool or multiple tools. This would also determine how the options are named. Similar is g.extension because it can list, add or remove.

imagery/i.signatures/i.signatures.html Outdated Show resolved Hide resolved
imagery/i.signatures/i.signatures.html Show resolved Hide resolved
Improve documentation and test to match current state
imagery/i.signatures/main.c Outdated Show resolved Hide resolved
@wenzeslaus wenzeslaus changed the title Imagery classifier signature management module i.signatures i.signatures: Add imagery classifier signature management module Jun 6, 2023
@marisn marisn requested a review from wenzeslaus June 6, 2023 16:37
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.

The C part looks good to go.

@marisn
Copy link
Contributor Author

marisn commented Sep 28, 2023

If there are no objections, I'll merge.

Thanks to parser, use of uninitalized variable was not possible,
but static code analizers are not aware of it.
@marisn marisn merged commit 34d02cd into OSGeo:main Oct 22, 2023
18 checks passed
@marisn marisn deleted the i_sig branch October 22, 2023 09:56
landam pushed a commit to landam/grass that referenced this pull request Oct 25, 2023
…eo#3008)

This module allows to manage signature files created by various imagery classification modules such as i.gensig, i.gensigset. The module allows to list, copy, rename and delete signature files.

---------

Co-authored-by: Anna Petrasova <kratochanna@gmail.com>
neteler pushed a commit to nilason/grass that referenced this pull request Nov 7, 2023
…eo#3008)

This module allows to manage signature files created by various imagery classification modules such as i.gensig, i.gensigset. The module allows to list, copy, rename and delete signature files.

---------

Co-authored-by: Anna Petrasova <kratochanna@gmail.com>
HuidaeCho pushed a commit to HuidaeCho/grass that referenced this pull request Jan 9, 2024
…eo#3008)

This module allows to manage signature files created by various imagery classification modules such as i.gensig, i.gensigset. The module allows to list, copy, rename and delete signature files.

---------

Co-authored-by: Anna Petrasova <kratochanna@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C Related code is in C
Development

Successfully merging this pull request may close these issues.

5 participants