Increase test coverage for fpdf.py

[RedShy]

I wrote some test to increase coverage for the file fpdf.py - #395
I wanted to write more but I encountered two problems:

• In the method ._putimage(), at line 3951 and 3994, in the context of image info, I see that the branch true of if info["cs"] == "Indexed": is not tested. I tried to make a test that follows the true branch, but I always see info["cs"] = "DeviceRGB". Then I see that in the module image_parsing there isn't the value "Indexed", but only "DeviceRGB" and "DeviceGray". So I conclude that if info["cs"] == "Indexed": cannot be ever accessed, maybe I'm missing something?
• In the method ._putfonts(), at line 3613, the branch true of if "type" in info and info["type"] != "TTF": is not tested. I tried to see which formats are supported other than "TTF" and I see that in the method add_font(), at line 1626, I see if ext not in (".otf", ".otc", ".ttf", ".ttc"):. I tried to open otf and ttc fonts but both seems not supported. Any suggestion to test this branch?

Checklist:

• The GitHub pipeline is OK (green),
  meaning that both pylint (static code analyzer) and black (code formatter) are happy with the changes of this PR.

• A unit test is covering the code added / modified by this PR

• This PR is ready to be merged

• In case of a new feature, docstrings have been added, with also some documentation in the docs/ folder

• A mention of the change is present in CHANGELOG.md

[codecov]

Codecov Report

Merging #439 (3d52b12) into master (f8b462c) will increase coverage by 0.32%.
The diff coverage is n/a.

@@            Coverage Diff             @@
##           master     #439      +/-   ##
==========================================
+ Coverage   91.55%   91.88%   +0.32%     
==========================================
  Files          22       22              
  Lines        6408     6408              
  Branches     1290     1290              
==========================================
+ Hits         5867     5888      +21     
+ Misses        311      298      -13     
+ Partials      230      222       -8     

Impacted Files	Coverage Δ
fpdf/ttfonts.py	85.73% <0.00%> (+0.27%)	⬆️
fpdf/fpdf.py	89.11% <0.00%> (+0.91%)	⬆️

---

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 f8b462c...3d52b12. Read the comment docs.

[Lucas-C]

In the method ._putimage(), at line 3951 and 3994, in the context of image info, I see that the branch true of if info["cs"] == "Indexed": is not tested. I tried to make a test that follows the true branch, but I always see info["cs"] = "DeviceRGB". Then I see that in the module image_parsing there isn't the value "Indexed", but only "DeviceRGB" and "DeviceGray". So I conclude that if info["cs"] == "Indexed": cannot be ever accessed, maybe I'm missing something?

Interesting. I think PyFPDF/fpdf2 used to support embedding images with indexed colors but the current code forces a conversion to RGB.
This makes things simpler, but maybe at a cost of slightly larger images embedded in the resulting PDF.
We have some unit tests that check this library behaviour with indexed ("palette") images here:

• https://github.com/PyFPDF/fpdf2/blob/master/test/image/png_images/README.md
• https://github.com/PyFPDF/fpdf2/blob/master/test/image/png_images/test_png_file.py

I think ideally we should not force conversion to RGB and allow users to embed indexed images, as the PDF spec permits it.

In the method ._putfonts(), at line 3613, the branch true of if "type" in info and info["type"] != "TTF": is not tested. I tried to see which formats are supported other than "TTF" and I see that in the method add_font(), at line 1626, I see if ext not in (".otf", ".otc", ".ttf", ".ttc"):. I tried to open otf and ttc fonts but both seems not supported. Any suggestion to test this branch?

Interesting again. Could that case be for "core" fonts? cf. https://github.com/PyFPDF/fpdf2/blob/2.5.4/fpdf/fpdf.py#L1734
The fact that the font file is read in that if makes me doubt this...

After digging a little more in the code history, I found out that valid values for type used to be also Type1 or TrueType:
https://github.com/PyFPDF/fpdf2/blob/1.7/fpdf/fpdf.py#L1130

I don't think that's possible anymore. Also, given our current plan in #418, I'm not sure it's really worth improving that part of the code, regarding clarity & coverage.

[RedShy]

Thank you for the fast and thoroughly reply!

I think ideally we should not force conversion to RGB and allow users to embed indexed images, as the PDF spec permits it.

I would like to see if I can insert this feature. It seems that the code is already there. I will try to make a new PR that implements it along with some tests

For the fonts parts I would like to get into them after the palette matter

[RedShy]

given our current plan in #418, I'm not sure it's really worth improving that part of the code, regarding clarity & coverage

Oh I see, so I agree with you, I think it's worth more focusing on the switch to using fonttools instead of trying to improve the clarity and coverage of that part of code! I would like to see if I can do something helpful there in the next days

For me this PR is concluded, it's up to you if you want to merge the unit tests I made here 😊

[Lucas-C]

Merged!
Thank you @RedShy 😊