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

Consistent approach to warnings #367

Open
aloukina opened this issue Feb 14, 2020 · 5 comments
Open

Consistent approach to warnings #367

aloukina opened this issue Feb 14, 2020 · 5 comments

Comments

@aloukina
Copy link
Collaborator

In some parts of the code we use logger.warningwhile in other parts we used warnings.
Is there any reason not use warnings everywhere?

@desilinguist
Copy link
Member

This is actually addressed in the Python logging HOWTO:

warnings.warn() in library code if the issue is avoidable and the client application should be modified to eliminate the warning.

logging.warning() if there is nothing the client application can do about the situation, but the event should still be noted.

@aloukina
Copy link
Collaborator Author

aloukina commented Feb 14, 2020

We have a few inconsistencies in how we apply this. For example we raise UserWarning when computing partial correlations. Another concern I have is that on user side both types of warnings look very similar so we are training the users to ignore all warnings.

@desilinguist
Copy link
Member

On a related note, we should be using FutureWarning for deprecation warnings and not DeprecationWarning since that's what the Python documentation recommends and this is what scikit-learn has recently done.

@desilinguist
Copy link
Member

@aloukina this is done right?

@aloukina
Copy link
Collaborator Author

No, not throughout the code ;(

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants