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

Make raster index usize #434

Merged
merged 1 commit into from
Nov 2, 2023
Merged

Make raster index usize #434

merged 1 commit into from
Nov 2, 2023

Conversation

lnicola
Copy link
Member

@lnicola lnicola commented Sep 6, 2023

  • I agree to follow the project's code of conduct.
  • I added an entry to CHANGES.md if knowledge of this change could be valuable to users.

@jdroenner
Copy link
Member

I'm not against this change but i always worry that there is a hidden reason why it is an int and not an uint in C/C++...

@lnicola
Copy link
Member Author

lnicola commented Sep 6, 2023

They're 1-based. I think it's just that int is the "default" data type in C.

nBandId the index number of the band to fetch, from 1 to GetRasterCount().

@lnicola
Copy link
Member Author

lnicola commented Sep 13, 2023

r? @jdroenner again 😃.

src/dataset.rs Outdated Show resolved Hide resolved
@metasim
Copy link
Contributor

metasim commented Oct 4, 2023

If it wouldn't cause a riot, I'd suggest we change it to NonZeroUsize. But it does add a bunch of unwrap and try_into friction.

@lnicola
Copy link
Member Author

lnicola commented Oct 7, 2023

Yeah, I've considered that, but it seems annoying. raster_count() would still return usize.

@metasim

This comment was marked as resolved.

@lnicola

This comment was marked as resolved.

@metasim
Copy link
Contributor

metasim commented Oct 30, 2023

@jdroenner FWIW I'm in favor of approving and merging this.

@jdroenner
Copy link
Member

i guess we need to resolve the changelog conflicts again

@lnicola
Copy link
Member Author

lnicola commented Nov 2, 2023

bors r=metasim,jdroenner

Copy link
Contributor

bors bot commented Nov 2, 2023

Build succeeded!

The publicly hosted instance of bors-ng is deprecated and will go away soon.

If you want to self-host your own instance, instructions are here.
For more help, visit the forum.

If you want to switch to GitHub's built-in merge queue, visit their help page.

@bors bors bot merged commit 2c1b33b into georust:master Nov 2, 2023
8 checks passed
@lnicola lnicola deleted the rasterband-usize branch November 2, 2023 10:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants