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

r.what: Update to handle more than 400 bands #4823

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

YannChemin
Copy link
Contributor

AVIRIS-NG has 425 bands, i.spectral is using r.what in the background has fails for hyperspectral imagery having bands more than 400.

AVIRIS-NG has 425 bands, i.spectral is using r.what in the background has fails for hyperspectral imagery having bands more than 400.
@github-actions github-actions bot added raster Related to raster data processing C Related code is in C module labels Dec 9, 2024
@YannChemin YannChemin changed the title Update main.c Update main.c of r.what to handle more than 400 bands Dec 9, 2024
@nilason nilason changed the title Update main.c of r.what to handle more than 400 bands r.what: Update to handle more than 400 bands Dec 9, 2024
Copy link
Member

@echoix echoix left a comment

Choose a reason for hiding this comment

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

Is there a specific reason why the number of files has a hardcoded limit? Is it something that should be documented?

@YannChemin
Copy link
Contributor Author

Is there a specific reason why the number of files has a hardcoded limit? Is it something that should be documented?

I do not know, 400 images was really a lot in the old times

@echoix
Copy link
Member

echoix commented Dec 9, 2024

Is there a specific reason why the number of files has a hardcoded limit? Is it something that should be documented?

I do not know, 400 images was really a lot in the old times

Now you showed a valid, real-world use case for more than 400 files!

@nilason nilason added this to the 8.5.0 milestone Dec 9, 2024
@nilason nilason added the backport to 8.4 PR needs to be backported to release branch 8.4 label Dec 9, 2024
@nilason
Copy link
Contributor

nilason commented Dec 9, 2024

Should probably be backported to 8.4.

@ninsbl
Copy link
Member

ninsbl commented Dec 9, 2024

Cool! There are several other use cases where overcoming that limitation is very useful. Just one question: Did you consider using ulimit instead of hardcoding another, higher fixed limit? r.series does something like that...
It could be handed down to other modules that use r.what (e.g. t.rast.what). In an case, documentation should be updated too, I would say...

@@ -18,7 +18,7 @@
* for details.
*
*****************************************************************************/
#define NFILES 400
#define NFILES 1000
Copy link
Member

@neteler neteler Dec 10, 2024

Choose a reason for hiding this comment

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

How about 1024 (I think a few hyperspectral cameras offering that).

Copy link
Member

@neteler neteler left a comment

Choose a reason for hiding this comment

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

The "TODO Fix 400 maps limit" needs to be removed from the HTML manual page file.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport to 8.4 PR needs to be backported to release branch 8.4 C Related code is in C module raster Related to raster data processing
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants