DEV: Introduce logger_warning

[MartinThoma]

No description provided.

[codecov]

Codecov Report

Merging #1148 (102b96f) into main (89c0ff2) will increase coverage by 0.02%.
The diff coverage is 81.44%.

@@            Coverage Diff             @@
##             main    #1148      +/-   ##
==========================================
+ Coverage   92.12%   92.14%   +0.02%     
==========================================
  Files          24       24              
  Lines        4772     4788      +16     
  Branches      989      990       +1     
==========================================
+ Hits         4396     4412      +16     
  Misses        228      228              
  Partials      148      148              

Impacted Files	Coverage Δ
PyPDF2/generic.py	91.72% <66.66%> (ø)
PyPDF2/_reader.py	90.35% <81.31%> (+0.16%)	⬆️
PyPDF2/_utils.py	98.78% <100.00%> (+0.02%)	⬆️

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update a6d27d7...102b96f. Read the comment docs.

[pubpub-zz]

@MartinThoma,
I'm not so found about using logger instead of warnings : you can no more convert log to exceptions that can then be used with post-mortem analysis ; I would propose you to use a local log definition(using logger by default is ok) than could then be overwritten using warning. Your opinion?

[MartinThoma]

In general this logic makes sense to me:

• exceptions for things the user should deal with (broken files that PyPDF2 cannot fix - or the strict mode)
• warnings for things the user can change in code (deprecations)
• logging warnings: things that help for issue analysis

you can no more convert log to exceptions that can then be used with post-mortem analysis

What do you mean by that?

I would propose you to use a local log definition

I'm sorry, I also don't understand that part. Could you please make an example 😅

[pubpub-zz]

add a function in _utils that will process the logs:

def logger_warning (msg,src):
       logging.getLogger(src).warn(msg)

where needed, you will call this function:
logger_warning("Illegal character in Name Object",__name__)
instead of
logger.warning("Illegal character in Name Object")

that way to convert logging to exception will just have to overload the function on request:

def mylog (msg,src):
    raise Exception(src+ " : "+msg)

PyPDF2._utils.logger_warning = mylog

[MartinThoma]

OK, I've now understood what you're suggesting. Thank you for going in detail ❤️

What I still don't understand is why you want to adjust it. Did you know that you can modify the log handler / log formatter of arbitrary logger objects?

We could even add the exception information: https://stackoverflow.com/a/29556246/562769

[MartinThoma]

I'm not so found about using logger instead of warnings

By the way: Thank you for giving me feedback ❤️ I'm always appreciating that 🤗

[pubpub-zz]

What I still don't understand is why you want to adjust it. Did you know that you can modify the log handler / log formatter of arbitrary logger objects?
eg: in #1091, the "errors" are reported through logger and you need to go in the module to convert the log to warning or exceptions. With my proposal debug would be eased 😊

[pubpub-zz]

What I still don't understand is why you want to adjust it. Did you know that you can modify the log handler / log formatter of arbitrary logger objects?
Using Logging Handlers could be a solution but complex to recover the stack in post-mortem 😗

[MartinThoma]

I still don't quite get it, but that's ok 😄 The proposal you made is a very good middle-ground:

• The users perspective: The exception/warnings.warn/logger.warning logic can still be as I want it to be (it's not consistent by now, but I'll probably make it consistent over time)
• The contributors perspective: You made the proposal, so I hope that's good :-)
• The maintainers perspective: The code still stays clean; I have to remember that part but it's not a complicated piece of code / it's very unlikely to cause other issues.

Give me a few minutes; I'll adjust the PR :-)