FAIRmat-NFDI / pynxtools

https://fairmat-nfdi.github.io/pynxtools/
Apache License 2.0
12 stars 8 forks source link

[Bug]: Informative stdout gets printed to stderr #292

Open mkuehbach opened 3 months ago

mkuehbach commented 3 months ago

Contact Details

markus.kuehbach@physik.hu-berlin.de

What happened?

Running pynxtools-apm using pynxtools from apm-em-tracking branch I spotted that some informative messages of the are directed to stderr although they should be directed to stdout or some other non-error-type stream.

Here an example from an otherwise finely parsing use case monitored using command line calling dataconverter convert [nothing special here some input files ...] 1>STDOUT.txt 2>STDERR.txt

STDERR.txt contained thereafter: Using apm reader to convert the given files:
• [...]/production/R18_53705-v03.epos • [...]/production/R18_53705-v03.rng • eln_data_apm.yaml • apm.oasis.specific.yaml The data entry corresponding to /ENTRY[entry]/definition/@version is required and hasn't been supplied by the reader. The output file generated: debug.aus_sydney_breen_1.nxs.

Only the line marked in bold I think warrants to be streamed to stderr as it is a warning, everything else in stdout.

mkuehbach commented 2 months ago

Inspected the situation further: As of today's master branch state log level is INFO stream is None i.e. the default stream which is stderr. So one can consider that info messages of the converter like above are indeed debug-type log messages for which we may use the interpretation that stderr is not only for errors so this is no bug, but:

In production runs now the only message indeed emitted by the converter unless there is at least a warning is alike to the above ones with always differing length. This avoids that one can check quickly just if the stderr content is 0 nothing so to skip to the next example instead of seeing some small byte payload that is always a similar message that (like the one above) that everything is right.

Two possible solutions: 1.) set streamHandle to point to sys.stdout , will achieve what is above stated but has the downside that also then everything more severe than INFO goes to stdout where it clearly should not be 2.) set log level to WARNING and move the above mentioned output to a print statement or another logger that writes to stdout.

I prefer solution 2.) as then there remains a life sign from the converter that it is doing something, relevant for NOMAD worker console and development work, but only severe issues get pumped into stderr.

What are your opinions about this @sherjeelshabih @sanbrock ?