search

Argument-Clinic / cpython

The Python programming language
https://www.python.org/
Other
1 stars 0 forks source link

PoC: Use logging for warnings #36

Open erlend-aasland opened 10 months ago

erlend-aasland commented 10 months ago

Another experiment: tear out the custom warning functionality, and instead use logging.warning.

Pro: simplified error handling code Con: no line numbers for warnings[^1]

[^1]: well, there were never usable line numbers for warnings anyway

erlend-aasland commented 10 months ago

Could be a nice yak-shave for #35

erlend-aasland commented 10 months ago

Another nice side effect is that post this, we could easily sprinkle Argument Clinic with debug and info log messages; it could significantly help improve the Argument Clinic debug experience.

erlend-aasland commented 9 months ago

What do you think, should we upstream this, @AlexWaygood? After this, it should be trivial to get rid of the global clinic object in the remaining fail().

vstinner commented 9 months ago

After this, it should be trivial to get rid of the global clinic object in the remaining fail().

Can you explain how this change make it easier to remove the global clinic object in fail()?

erlend-aasland commented 9 months ago

Can you explain how this change make it easier to remove the global clinic object in fail()?

warn() and fail() currently share implementation in warn_or_fail; tearing them out one at the time makes for better targeted PRs that are easier to review, and have smaller diffs.

vstinner commented 9 months ago

But how do you avoid access clinic global variable thanks to the logging module? You still need to retrieve filename and line number location somehow, no?

erlend-aasland commented 9 months ago

But how do you avoid access clinic global variable thanks to the logging module? You still need to retrieve filename and line number location somehow, no?

We never had line numbers for warn(); they are called from places in clinic.py where we don't have the line number at hand. IMO, it is better to acknowledge that fact, and just produce the warning. We have the filename, as you see from the PR :)

vstinner commented 9 months ago

FYI I wrote https://github.com/python/cpython/pull/114752 to remove the usage global variable clinic.