Skip to content

Annotation for getLevelName in logging does not accept str but the code does #1842

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.

Sign up for GitHub

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
Closed
gabbard opened this issue Jan 25, 2018 · 3 comments
Closed

Annotation for getLevelName in logging does not accept str but the code does #1842

gabbard opened this issue Jan 25, 2018 · 3 comments

Comments

@gabbard
Copy link

gabbard commented Jan 25, 2018

Currently for getLevelName in logging there is

def getLevelName(lvl: int) -> str: ...

However the code for it (Python 3.6.3) accepts a string argument as well:

def getLevelName(level):
    """
    Return the textual representation of logging level 'level'.

    If the level is one of the predefined levels (CRITICAL, ERROR, WARNING,
    INFO, DEBUG) then you get the corresponding string. If you have
    associated levels with names using addLevelName then the name you have
    associated with 'level' is returned.

    If a numeric value corresponding to one of the defined levels is passed
    in, the corresponding string representation is returned.

    Otherwise, the string "Level %s" % level is returned.
    """
    # See Issues #22386, #27937 and #29220 for why it's this way
    result = _levelToName.get(level)
    if result is not None:
        return result
    result = _nameToLevel.get(level)
    if result is not None:
        return result
    return "Level %s" % level

It is not clear to me from the method's docstring that the behavior of the code with respect to strings is guaranteed, though, so perhaps leaving it as int is safer.
@gvanrossum
Copy link
Member

gvanrossum commented Jan 26, 2018 via email

@gabbard
Copy link
Author

gabbard commented Jan 29, 2018

Thanks - since the "reverse" behavior is considered a bug, typeshed is definitely both correct and helpful here.

I think the fundamental problem, though, is that it is difficult for the user (especially someone new to Python) to determine whether or not the reverse behavior in fact is a bug. There are many examples of the deprecated behavior on the web, the code supports the reverse behavior without a comment that its use is deprecated, and the docstring doesn't contain the "Changed in version 3.4... message" (only the RST docs do, and many inexperienced users would be unaware they need to check both places - you wouldn't for the Java Standard Library, for example). There is a comment # See Issues #22386, #27937 and #29220 for why it's this way, but even if the user tracks down those issues, none of them clearly say the reverse behavior should be considered a bug, and the last even contains the comment "I perhaps should have done a separate API for that, like getLevelForName(), but it's too late now due to backward compatibility requirements - it's been like this for a long time", which implies that the reverse usage is the correct (and perhaps only available) way of obtaining the desired behavior.

All of that isn't typeshed's problem, of course - is https://bugs.python.org/ the correct place to request such an update to the docstring to clarify the reverse behavior is deprecated?

@gabbard gabbard closed this as completed Jan 29, 2018
@gvanrossum
Copy link
Member

gvanrossum commented Jan 29, 2018 via email

@jenshnielsen jenshnielsen mentioned this issue Oct 29, 2018
Merged
cjwatson added a commit to cjwatson/typeshed that referenced this issue Jun 3, 2024
@cjwatson
Tighten annotation of logging.getLevelName
4651635 
To better reflect the implementation's behaviour,
python#2730 changed
`logging.getLevelName` to accept `int | str` and return `Any` (the
latter due to the need to avoid union return types).  However, this
isn't ideal if you're passing in an `int`, in which case the
implementation always returns a `str`.  Add overloads for this.

This is all arguably a bit unfortunate in light of
python#1842 (comment),
but I don't want to relitigate that here.  I've at least left a comment.
@cjwatson cjwatson mentioned this issue Jun 3, 2024
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
2 participants
@gvanrossum @gabbard