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

Use consistent style for warnings #418

Open
zzril opened this issue Jul 4, 2023 · 1 comment
Open

Use consistent style for warnings #418

zzril opened this issue Jul 4, 2023 · 1 comment
Labels
cleanup 🧹 Refactorings and other tasks that improve the code good first issue Good for newcomers

Comments

@zzril
Copy link
Contributor

zzril commented Jul 4, 2023

Is your feature request related to a problem?

The way warnings are raised is not consistent throughout the codebase.

Some things I noticed:

  1. _elastic_net_regression.py uses both from warnings import warn (in combination with calls to warn(...)) and import warnings (in combination with warnings.warn(...)).
  2. Sometimes, UserWarning is explicitely passed as category keyword argument, sometimes it is left out (as UserWarning is the default value).
  3. The warnings raised in the Transformer classes lack the trailing period.
  4. The warnings raised in the Transformer class generally talk about columns even when there could be just one column that is mentioned. Saying column(s) might be more accurate in those cases.

Point 3 is actually somewhat important because it would make writing regexes easier if the warnings all had the same format.
Point 4 is probably the least important, might even decide to keep it as is, as the messages are probably easier to read this way.


To quickly find the warnings raised with warnings.warn:

grep -E -r -n 'warnings.warn' src

To quickly find those raised with warn:

grep -E -r -n 'from warnings import warn' src

Desired solution

Use the same style everywhere.

Ideally, find a way to have the linter enforce a consistent style.

@zzril zzril added good first issue Good for newcomers cleanup 🧹 Refactorings and other tasks that improve the code labels Jul 4, 2023
@github-project-automation github-project-automation bot moved this to Backlog in Library Jul 4, 2023
@lars-reimann
Copy link
Member

lars-reimann commented May 4, 2024

stacklevel should also always be set to 2.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cleanup 🧹 Refactorings and other tasks that improve the code good first issue Good for newcomers
Projects
Status: Backlog
Development

No branches or pull requests

2 participants