uafgeotools / waveform_collection

Collect seismic/infrasound waveforms and metadata from IRIS/WATC/AVO servers, local miniSEED files, etc.
https://uaf-waveform-collection.readthedocs.io/
MIT License
11 stars 6 forks source link

Add a "verbose" boolean argument to gather_waveforms #27

Closed darren-tpk closed 1 year ago

darren-tpk commented 1 year ago

Just a QOL suggestion. I often have gather_waveforms in big loops for long-term processes, and the lines that it prints out on every call takes up big chunks of space in my output log file. I do think the CollectionWarning should be retained, though. But it would be useful to shut off the "GATHERING DATA" lines for looped calls.

liamtoney commented 1 year ago

This could be a useful feature, and perhaps a good way to start using logging. Wanna make a new branch + PR?

darren-tpk commented 1 year ago

I was initially thinking of just using sys and os to block calls to print and warnings.filterwarnings('ignore') to suppress warnings. That way the option is straightforward with a boolean argument when the gather_waveforms is called. logging does seem more versatile with the levels option, but I feel like it might introduce too many options. I see its use for debugging and active development purposes, but it might not be necessary here. Either way, happy to make a new branch to incorporate this utility.

darren-tpk commented 1 year ago

Sorry, I closed the issue by accident.

liamtoney commented 1 year ago

I was initially thinking of just using sys and os to block calls to print and warnings.filterwarnings('ignore') to suppress warnings.

Using warnings.filterwarnings() for warnings makes sense. But I'm not so sure about the sys and os solution for print(), it seems pretty hacky to me. And seems like more lines of code, kind of like having a

if verbose:
    print('Blah')

conditional flow.

I believe that replacing all calls to print() with logging.info() should do the trick, no? Then the boolean flag would simply say logging.disable() or whatever the command is. Hence the more complicated functionality such as level can be avoided. And our print statements don't need to be wrapped in a conditional or other additional code.

Does that make sense?

Helpful table here: https://docs.python.org/3/howto/logging.html#when-to-use-logging

liamtoney commented 1 year ago

And seems like more lines of code, kind of like having a

if verbose:
    print('Blah')

conditional flow.

Okay, I see how you just start blocking at the beginning of the function and then stop it at the end. But reading the SO thread it sounds like the only safe way to do this is within a context manager since otherwise printing may stay disabled if the function errors out.

tl;dr I still think logging should be a quick and easy improvement!

liamtoney commented 1 year ago

But reading the SO thread it sounds like the only safe way to do this is within a context manager since otherwise printing may stay disabled if the function errors out.

Sigh. I'm realizing this is true of the logging approach as well.