ratal / mdfreader

Read Measurement Data Format (MDF) versions 3.x and 4.x file formats in python
Other
169 stars 74 forks source link

Printing warnings to the console #133

Closed cristi-neagu closed 6 years ago

cristi-neagu commented 6 years ago

Hello,

This is to discuss a solution (if a solution is even required) to the various warnings printed by the library. This often interfere with scripts, ruining output formating. In this case, messing up a progress bar:

11%|████▊                                      | 4/36 [00:01<00:08,  3.61it/s]no data to be resampled
no data to be resampled
 17%|███████▏                                   | 6/36 [00:01<00:06,  4.44it/s]no data to be resampled
 25%|██████████▊                                | 9/36 [00:02<00:06,  4.36it/s]no data to be resampled
 31%|████████████▊                             | 11/36 [00:02<00:05,  4.33it/s]no data to be resampled
 44%|██████████████████▋                       | 16/36 [00:03<00:04,  4.24it/s]no data to be resampled
 58%|████████████████████████▌                 | 21/36 [00:05<00:03,  3.94it/s]no data to be resampled
 61%|█████████████████████████▋                | 22/36 [00:05<00:03,  4.02it/s]no data to be resampled
 64%|██████████████████████████▊               | 23/36 [00:05<00:03,  4.04it/s]no data to be resampled
 67%|████████████████████████████              | 24/36 [00:05<00:02,  4.06it/s]no data to be resampled
 72%|██████████████████████████████▎           | 26/36 [00:06<00:02,  4.12it/s]no data to be resampled
 86%|████████████████████████████████████▏     | 31/36 [00:07<00:01,  4.06it/s]no data to be resampled
 89%|█████████████████████████████████████▎    | 32/36 [00:07<00:00,  4.10it/s]no data to be resampled
 97%|████████████████████████████████████████▊ | 35/36 [00:08<00:00,  4.13it/s]no data to be resampled
100%|██████████████████████████████████████████| 36/36 [00:08<00:00,  4.14it/s]

The particular library i am using (tqdm) has ways of dealing with print statements inside the loop by routing the output to a custom print function, but that means changing the print instruction itself. While i supposed that can be done inside mdfreader to deal with this particular progress bar library, i am certain it won't work in all cases.

I would argue that the best way to handle these types of messages is by returning a status message. In the case of mdfreader.resample(), the function could return the error message. If i am interested in knowing if there is any data to resample or not, i would store the result in a variable. If not, i can just not store it. This would not need any code changes in any function using this library. This might be difficult to implement seamlessly in functions which already return values (such as .getChannelData())

Another approach would be to have some sort of flag that puts the library in "quiet" mode. This would have the advantage of working seamlessly with all functions.

Sometimes, these warnings are expected and should be dealt with in code. If there is no data to resample and my script doesn't handle this correctly, it will still crash whether or not i get the warning.

Let me know what you think about this. It will obviously take some time to implement, but i think it's a good change.

ratal commented 6 years ago

Yes, this is something which is not so clean. All the prints that are annoying you, are actually there to inform user something happened during parsing but it is not important enough to stop processing and raising error. Most of them are actually rather there for debugging purpose. Error management is also weak I think, but another subject. I could propose to actually convert all those prints by warnings.warn('message'). Then you could tune behaviour using filter from warnings module. Already existing information to user remains and you can suppress it when you need.

cristi-neagu commented 6 years ago

That sounds fair enough. A little bit roundabout, but it's probably the pythonic way of doing it.

Edit: For this to work properly, the raised warnings should be specific to mdfreader, so that only they can be filtered out.

ratal commented 6 years ago

Should be done in dev branch, you could try.

cristi-neagu commented 6 years ago

I can confirm that it does seem to work at least for the particular issue i was having. To hide the warnings, this is the code i used, roughly:

import mdfreader as mdf
import warnings

datFile = mdf.mdf('test.dat', channelList=[])
with warnings.catch_warnings():
    warnings.simplefilter('ignore')
    datFile.resample(0.1)

Note that i am loading the file with an empty channelList to force the warnings.

I'd say that solved, for now. Thank you.