BUG: Writing German characters into form fields

[pubpub-zz]

closes #2035
closes #2021

[codecov]

Codecov Report

Patch coverage: 66.66% and project coverage change: -0.11% ⚠️

Comparison is base (fb4f466) 94.28% compared to head (d4cb793) 94.17%.

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #2047      +/-   ##
==========================================
- Coverage   94.28%   94.17%   -0.11%     
==========================================
  Files          41       41              
  Lines        7364     7386      +22     
  Branches     1451     1458       +7     
==========================================
+ Hits         6943     6956      +13     
- Misses        262      267       +5     
- Partials      159      163       +4     

Files Changed	Coverage Δ
pypdf/_writer.py	87.62% <62.96%> (-0.56%)	⬇️
pypdf/_cmap.py	94.40% <100.00%> (+0.06%)	⬆️

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[pubpub-zz]

Test converage is not wonderfull but I miss ideas

@MartinThoma all yours

[pubpub-zz]

PS : I will need this PR for a new one (which will deal with full unicode(arabic and asian chararacter sets) ) 😉

[pubpub-zz]

stdby

[MartinThoma]

The code after the exception will never be executed

I would use ValueError instead of AssertionError: https://docs.python.org/3/library/exceptions.html#exception-hierarchy - but I cannot give a good reason why 🤔

[stefan6419846]

I agree that using AssertionErrors usually is a bad approach. Why raising an exception at all when there apparently is a way to continue afterwards after having logged a (recoverable) violation/issue?

[pubpub-zz]

The assertion error is here to detect issues when using pytest. In operational condition it is the logger_warning (non blocking) that should be raised

[MartinThoma]

I don't understand that. The Exception will also be raised in prod code, not only in pytest.

When you execute the following with python:

print("First")
raise AssertionError("the exception")
print("Second")

you get:

First
Traceback (most recent call last):
  File "/home/moose/foo.py", line 2, in <module>
    raise AssertionError("the exception")
AssertionError: the exception

[pubpub-zz]

I see my mistake. I originally used assert but ruff changed to the exception only.
I propose this code:

if __debug__:
            raise AssertionError("can not find font dictionary")
else:
            logger_warning(f"can not find font dictionary for {font_name}", __name__)

your opinion ?

[MartinThoma]

Interesting, I didn't know about __debug__ - thanks for sharing 🙏

Sadly, I think it cannot be used like that. I think more than 90% of all users don't use/know the -O flag.

[MartinThoma]

Within pytest, we can make warnings which are not captured via with pytest.warns be treated as errors with pytest -W error: https://docs.python.org/3/using/cmdline.html#cmdoption-W

[MartinThoma]

I think that is probably a good idea as default as well 🤔

[pubpub-zz]

within pyproject.toml there is already:

[tool.pytest.ini_options]
addopts = "--disable-socket"
filterwarnings = ["error"]

isn't this sufficient consider warnings as errors ?
for the moment I removed the raise line

[MartinThoma]

Yes, I think that should do it. Didn't it work?

[pubpub-zz]

Coverage should improve with further PRs
@MartinThoma all yours

[pubpub-zz]

conflict solved