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

Printing warnings to the console #133

Closed
cristi-neagu opened this issue Feb 16, 2018 · 4 comments
Closed

Printing warnings to the console #133

cristi-neagu opened this issue Feb 16, 2018 · 4 comments

Comments

@cristi-neagu
Copy link
Contributor

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
Copy link
Owner

ratal commented Feb 16, 2018

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
Copy link
Contributor Author

cristi-neagu commented Feb 16, 2018

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
Copy link
Owner

ratal commented Feb 16, 2018

Should be done in dev branch, you could try.

@cristi-neagu
Copy link
Contributor Author

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.

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

No branches or pull requests

2 participants