cogeotiff / rio-cogeo

Cloud Optimized GeoTIFF creation and validation plugin for rasterio
https://cogeotiff.github.io/rio-cogeo/
BSD 3-Clause "New" or "Revised" License
308 stars 42 forks source link

All Console output is standard error #246

Closed im28 closed 1 year ago

im28 commented 1 year ago

Good day, Thank you for this great tool, I'm calling rio cogeo from another application, I would like to know why any output of the program is flagged as a standard error. for example https://github.com/cogeotiff/rio-cogeo/blob/45b462e9503d29fe077b27a35d4b0e517c9fa76b/rio_cogeo/cogeo.py#L294

vincentsarago commented 1 year ago

👋 Hi @im28, thanks for opening the issue.

I guess that's just a bad habit I had to avoid considering output from the CLI as proper output. I'm fine removing this and will be happy to review any PR 🙏

kylebarron commented 1 year ago

As another opinion, I agree with this article that all informational updates, like in this context, should go to stderr, not stdout.

Standard output is where your script main's output should go. For instance, the command ls is responsible for listing the content of a directory. This listing goes to stdout.

Standard error is where debugging information and errors should go. The name is a bit misleading because you can use it to display runtime informations and not only errors.

The main usefulness of printing informative updates to stderr is that piping to other CLI programs works better because only the actual data is on stdout.

vincentsarago commented 1 year ago

reading the article shared ☝️ (🙏 @kylebarron) I think using stderr is fine then. @im28 you can use the quiet option to remove any messages

im28 commented 1 year ago

Thank you @kylebarron for sharing the article ❤. I guess then rio cogeo create has no standard output due to the output being a file, that's understandable. Unfortunately, this means there is no way to obtain progress information whilst maintaining separation between actual errors and other information.

Would it be possible to add an optional callback parameter to cog_translate API? The callback would be called to report progress. Perhaps something like (this is just a draft for the idea, the callback signature is probably going to be different)

def on_progress(message:string, percentage_complete:float):
    """Callback function to show on UI"""
    print(message)
    print(percentage_complete:float)

cog_translate(on_progress_callback=on_progress)

If the proposal is okay for you, I would be happy to work on a PR.

kylebarron commented 1 year ago

I'm not sure I understand your use case still. What kind of program are you calling rio cogeo from? Non-python through the CLI? If it's via a shell process the main way you'd usually find if it errored is through a non-zero exit code, not by just checking whether text exists on stderr

im28 commented 1 year ago

@kylebarron, Thanks for the explanation, Apologies for not being clear enough. I have a python script that calls the cog_translate API among other things, (this script is later called through the CLI via C#).

If it's via a shell process the main way you'd usually find if it errored is through a non-zero exit code

I didn't think of that, I believe that will work for my use case. I will close this issue. Thank you!

I still believe having a progress callback might be a nice addition perhaps I should open another issue for that, let me know your thoughts.