The Wayback Machine - http://web.archive.org/web/20220318041649/https://github.com/taichi-dev/taichi/issues/4543
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

Jump to bottom

taichi.lang.util.warning doesn't respect stdlib warnings filters #4543

Open

RuRo opened this issue Mar 15, 2022 · 4 comments
Open

taichi.lang.util.warning doesn't respect stdlib warnings filters #4543

RuRo opened this issue Mar 15, 2022 · 4 comments
Labels
good first issue welcome contribution
Projects
Codebase Quality

Comments

@RuRo
Copy link
Contributor

@RuRo RuRo commented Mar 15, 2022

The taichi.lang.util.warning function just prints the warning without consulting the current state of pythons standard library warnings module.

For example:

import warnings
import taichi as ti

with warnings.catch_warnings():
    warnings.simplefilter("ignore")
    ti.lang.util.warning("Oh, no!") # This line shouldn't print anything

with warnings.catch_warnings():
    warnings.simplefilter("error")
    ti.lang.util.warning("Oh, no!") # This line should raise the warning as an exception

You should either

  1. use warnings.warn instead of print (preferable, imho)
  2. reimplement the standard filtering logic (see here)

Additionally, ti.lang.util.warning prints the errors to stdout instead of the stderr, which is also wrong.
@strongoier
Copy link
Member

@strongoier strongoier commented Mar 16, 2022

Thanks for pointing this out! Option 1 looks good to me.
wdyt @ailzhang @frostming

@strongoier strongoier added this to To do in Codebase Quality via automation Mar 16, 2022
@frostming
Copy link
Collaborator

@frostming frostming commented Mar 16, 2022 •
edited

I agree to use the standard warnings.warn and Warning subclasses. Or remove this function entirely(to use the standard warnings.warn)

@ailzhang
Copy link
Collaborator

@ailzhang ailzhang commented Mar 16, 2022

Option 1 sounds great to me! @RuRo would you mind sending a PR to update this function? Thanks a lot!

@RuRo
Copy link
Contributor Author

@RuRo RuRo commented Mar 17, 2022

Hi, sorry for the delayed reply. Unfortunately, I am kind of busy lately, so I won't be able to contribute anything in the forceable future. I'll keep submitting issues as I run into them while using taichi and I might revisit them once I get some free time. No promises tho, so if anyone wants to contribute it, that would be great.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
good first issue welcome contribution
Projects
Codebase Quality
To do
Milestone
No milestone
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
4 participants
@strongoier @RuRo @ailzhang @frostming