STINNER Victor <vstin...@python.org> added the comment:

Vinay:
> That code in the wild that sets the level attribute directly is wrong and 
> should be changed, right? The documentation has always been clear that 
> setLevel() should be used. If we now take steps to support setting the level 
> via attribute, isn't that encouraging bypassing the documented APIs? I'm not 
> sure such misuse is widespread.

I understood that Zane Bitter opened this issue *because* people misuses the 
logging API.


Vinay:
> (...) Sure, mistakes will happen, and that's the price paid when one doesn't 
> follow documentation. It feels wrong to do this as a band-aid to help out 
> people who didn't do the right thing.

setLevel() enforces consistency: it calls _checkLevel() to convert string to 
integer. Also, Logger.setLevel() also invalidates caches.

Having a way to update the level without invalidating caches sounds like a bug 
to me.

--

Zane proposed to deprecate setting the level attribute in PR 15286. I dislike 
this idea. It's kind of weird to be able to *read* a public attribute but not 
to *set* it. Either the attribute should be removed by adding a getLevel() 
method (and deprecate the attribute and then remove it), or it should be 
possible to get and set it.

I'm in favor of not deprecating "logger.level = level". Just wrap it into a 
property silently.

--

I rewrote the microbenchmark using pyperf:
---
import pyperf

class Logger(object):
    def __init__(self):
        self._levelprop = self.level = 0

    @property
    def levelprop(self):
        return self.level


logger = Logger()
ns = {'logger': logger}
runner = pyperf.Runner()
runner.timeit('read attribute', 'logger.level', globals=ns)
runner.timeit('read property', 'logger.levelprop', globals=ns)
---

Result:

* read attribute: Mean +- std dev: 38.9 ns +- 0.8 ns
* read property: Mean +- std dev: 104 ns +- 3 ns

Reading a property is 2.7x slower than reading an attribute. Well, that's not 
surprising to have an overhead. But the absolute numbers remain reasonable. 
We're talking about 100 ns, not 100 ms.

IMHO the overhead is reasonable in 3rd party code. Inside the logging module, 
only the private _level attribute should be used and so there is no overhead.

----------
nosy: +vstinner

_______________________________________
Python tracker <rep...@bugs.python.org>
<https://bugs.python.org/issue37857>
_______________________________________
_______________________________________________
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

Reply via email to