Skip to content

Conversation

NN---
Copy link

@NN--- NN--- commented Jan 8, 2025

@ghost
Copy link

ghost commented Jan 8, 2025

All commit authors signed the Contributor License Agreement.
CLA signed

@bedevere-app
Copy link

bedevere-app bot commented Jan 8, 2025

Most changes to Python require a NEWS entry. Add one using the blurb_it web app or the blurb command-line tool.

If this change has little impact on Python users, wait for a maintainer to apply the skip news label instead.

@bedevere-app
Copy link

bedevere-app bot commented Jan 8, 2025

Most changes to Python require a NEWS entry. Add one using the blurb_it web app or the blurb command-line tool.

If this change has little impact on Python users, wait for a maintainer to apply the skip news label instead.

@bedevere-app
Copy link

bedevere-app bot commented Jan 9, 2025

Most changes to Python require a NEWS entry. Add one using the blurb_it web app or the blurb command-line tool.

If this change has little impact on Python users, wait for a maintainer to apply the skip news label instead.

@bedevere-app
Copy link

bedevere-app bot commented Jan 9, 2025

Most changes to Python require a NEWS entry. Add one using the blurb_it web app or the blurb command-line tool.

If this change has little impact on Python users, wait for a maintainer to apply the skip news label instead.

@bedevere-app
Copy link

bedevere-app bot commented Jan 9, 2025

Most changes to Python require a NEWS entry. Add one using the blurb_it web app or the blurb command-line tool.

If this change has little impact on Python users, wait for a maintainer to apply the skip news label instead.

@NN--- NN--- changed the title Introduce qualname in extra parameters providing qualified name including classname. gh-128688 Introduce qualname in extra parameters providing qualified name including classname. Jan 9, 2025
@NN--- NN--- marked this pull request as ready for review January 9, 2025 19:57
@NN--- NN--- requested a review from vsajip as a code owner January 9, 2025 19:57
@bedevere-app
Copy link

bedevere-app bot commented Jan 9, 2025

Most changes to Python require a NEWS entry. Add one using the blurb_it web app or the blurb command-line tool.

If this change has little impact on Python users, wait for a maintainer to apply the skip news label instead.

@NN--- NN--- requested a review from methane January 9, 2025 19:57
@picnixz
Copy link
Member

picnixz commented Jan 10, 2025

This requires a NEWS entry though. This is a new feature so we need a blurb and a What's New entry (in Doc/whatsnew/3.14.rst). This also requires the logging documentation to be updated.

@StanFromIreland
Copy link
Member

Maybe update the docstring with this change?

- Find the stack frame of the caller so that we can note the source
- file name, line number and function name.
+ Find the stack frame of the caller so that we can note the source
+ file name, line number and function name (qualified name if
+ `self.use_qualname`  is ``True`` , otherwise simple name).

@vsajip
Copy link
Member

vsajip commented Jan 10, 2025

Well, it's not enough to update the docstring - the documentation should be updated (lots of people using the package wouldn't be looking at the source code), and also unit tests are needed.

@vsajip
Copy link
Member

vsajip commented Jan 16, 2025

Also, I noticed that the title of this PR, and the discussion on DPO ( discuss.python.org) (which got no traction), both mention passing the qualname as an extra parameter - but that's not what this PR does. Also, your code only works at an instance level, which means that you'd have to set it for every logger instance individually - doesn't seem very friendly. You'd need to ensure that the attribute can also be set by configuration rather than only programmatically.

To my mind, the process of proposing a new feature by a non-maintainer should be:

  1. Propose the feature on DPO and see if people think it will be generally useful.
  2. If deemed useful by enough people, raise an issue and optionally a PR.

Although the qualified name isn't provided (I guess as it wasn't available in Python < 3.11), it's not strictly necessary, as the line number and pathname in log output can locate the logging location exactly. It might be seen as more friendly, but then people have to worry about what happens when their code runs on Python < 3.11.

Have you thought these kinds of things through? If so, what have you concluded?

@NN---
Copy link
Author

NN--- commented Jan 16, 2025

Sorry for not answering, was too busy.
You have good points.
The change log is indeed important.
I wanted to understand whether this feature is useful not just for me and my colleagues.

The line unfortunately is not enough and the function name too as we have sometimes big files with different classes implementing same interface so all the function names are the same.

Regarding the per instance vs per class setting is a good point as well.
Probably both are useful to maintain backward compatibility while allowing gradually log change.

Everything will be addressed in the near time.

Thank you for your comments.

@vsajip
Copy link
Member

vsajip commented Jan 17, 2025

The line unfortunately is not enough and the function name too as we have sometimes big files with different classes implementing same interface so all the function names are the same.

This sounds like a problem with your codebase rather than a more general problem, and the standard library is for general improvements rather than things which only benefit a narrow category of use cases. Anyway, I still don't see why a combination of pathname and line number don't pinpoint the code location exactly in your situation.

@NN---
Copy link
Author

NN--- commented Jan 18, 2025

It does pinpoint to the place in the code.
Having qualified name in log makes it much easier for many people.

The proposed feature is opt-in, so only ones who need it will turn it on.

@vsajip
Copy link
Member

vsajip commented Jan 19, 2025

The proposed feature is opt-in, so only ones who need it will turn it on.

The bar for adding features to the stdlib is high, even for opt-in features. It's got to be useful for a fair number of people and maintain backwards compatibility.

@NN---
Copy link
Author

NN--- commented Jan 19, 2025

What is the best way to understand whether many people enough find this useful?

@picnixz
Copy link
Member

picnixz commented Jan 19, 2025

What is the best way to understand whether many people enough find this useful?

Open a thread on https://discuss.python.org/c/ideas/6. If there's enough support, we can accept it. Otherwise, if there's no consensus, it's generally something that shouldn't be part of the standard library.

My 2c: adding the qualname in addition to the filename + lineno is sometimes easier because you can CTRL+F the qualname. I personally don't need it though.

@NN--- NN--- closed this Jan 19, 2025
@NN---
Copy link
Author

NN--- commented Jan 19, 2025

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants