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

Add Progress Option #273

Merged
merged 3 commits into from
Dec 11, 2023
Merged

Add Progress Option #273

merged 3 commits into from
Dec 11, 2023

Conversation

SellersEvan
Copy link
Contributor

I am working on a project that utilized rio-cogeo and we needed to be able to get the progress of a conversion. I thought other people might also have this issue. Basically, what this new feature allows you to do is set a TextIO output (like sys.stdout) to output the progress as a percentage to (and just the percentage). This would allow something else to read this TextIO to get the percentage that the conversion is at. This can be extremely useful to allow developers to check the progress of an update.

Let me know if you have any questions.

@SellersEvan
Copy link
Contributor Author

@vincentsarago Does this look something the team would be interested in?

@vincentsarago
Copy link
Member

sorry @SellersEvan I was busy on other projects.

To be honest the proposed solution might be a bit too complex at the moment, especially that you're calling 4 times updateProgress with the same percent.

would an option to overwrite fout

fout = ctx.enter_context(open(os.devnull, "w")) if quiet else sys.stderr
be possible?

@vincentsarago
Copy link
Member

worth noting that the percentage here does not reflect the status of the COG translation but only the reading part

@SellersEvan
Copy link
Contributor Author

@vincentsarago

I removed the code complexity from the last update and instead opted to override the fout like you suggested. This adds some complexity to using it because Click will only output the progress if the output file is interactive, like a terminal. This can be hacked however, by overriding the text buffers isatty method to always return true - this is documented in the API examples.

Thoughts?

@@ -92,7 +92,7 @@ def cog_translate( # noqa: C901
allow_intermediate_compression: bool = False,
forward_band_tags: bool = False,
forward_ns_tags: bool = False,
quiet: bool = False,
quiet: Union[bool, TextIO] = False,
Copy link
Member

Choose a reason for hiding this comment

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

I don't think we should use quiet for both.

we can add another option (name tbd)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

We can keep quite as it was before and add a new option maybe called progress or progress_out. I image the logic being like this...

fout = ctx.enter_context(open(os.devnull, "w")) if quiet else sys.stderr
if quiet is False and progress_out:
    fout = progress_out

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Implemented. Ready for review.

@vincentsarago vincentsarago merged commit 3ae8081 into cogeotiff:main Dec 11, 2023
5 checks passed
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