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

Coverage fraction #40

Closed
wants to merge 14 commits into from
Closed

Conversation

hellkite500
Copy link
Contributor

This PR introduces two mechanics for getting coverage fraction reports from the core lib -- it can be asked for as an operation alongside other stats, or from the CLI it can be provided as a strategy which will only write the coverage fraction and cell number.

Tests coverage was added to the catch tests for the coverage fraction.

Am happy to discuss any implementation details.

@hellkite500 hellkite500 mentioned this pull request Jan 31, 2023
@dbaston
Copy link
Member

dbaston commented Oct 5, 2023

I'm working through this at long last, @hellkite500 . Are you OK with me pushing commits to this branch?

it can be asked for as an operation alongside other stats, or from the CLI it can be provided as a strategy which will only write the coverage fraction and cell number.

I'm inclined to retain the second approach only, rather than modify the first approach to only store the coverage fractions in RasterStats if we need them. This more closely resembles the R interface, where you can get either stats or the raw coverage fractions, but not both in the same call. Since calculating the coverage fractions is usually quick relative to I/O I wouldn't predict a strong performance advantage to outputting stats and coverage fractions in a single pass.

@dbaston
Copy link
Member

dbaston commented Oct 29, 2023

Adapted, committed in e943d7d. Thanks!

@dbaston dbaston closed this Oct 29, 2023
@hellkite500
Copy link
Contributor Author

Sorry I missed the notification that you were working on this! Glad it was useful in some capacity!

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.

2 participants