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

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

Closed
RuRo opened this issue Mar 15, 2022 · 6 comments
Closed

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

RuRo opened this issue Mar 15, 2022 · 6 comments
Labels
good first issue A great chance for starters welcome contribution

Comments

@RuRo
Copy link
Contributor

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.

@RuRo RuRo added the potential bug Something that looks like a bug but not yet confirmed label Mar 15, 2022
@strongoier
Copy link
Contributor

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 commented Mar 16, 2022

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
Contributor

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 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.

@frostming frostming added welcome contribution good first issue A great chance for starters and removed potential bug Something that looks like a bug but not yet confirmed labels Mar 18, 2022
@pigletfly
Copy link

I'd like to take a shot on this @ailzhang

@ailzhang
Copy link
Contributor

@pigletfly Awesome it's yours! Please don't hesitate to ping us here if you have any questions!

@gingerkidney gingerkidney removed their assignment Dec 6, 2022
ailzhang pushed a commit that referenced this issue Feb 8, 2023
…l.warning (#7301)

Issue: #4543

### Brief Summary
Changed print statement in `taichi.lang.util.warning` to `warnings.warn`
to respect stdlib warnings filters

---------

Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>
Codebase Quality automation moved this from To do to Done Apr 14, 2023
quadpixels pushed a commit to quadpixels/taichi that referenced this issue May 13, 2023
…l.warning (taichi-dev#7301)

Issue: taichi-dev#4543

### Brief Summary
Changed print statement in `taichi.lang.util.warning` to `warnings.warn`
to respect stdlib warnings filters

---------

Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
good first issue A great chance for starters welcome contribution
Projects
Development

No branches or pull requests

7 participants