Override heading style with supplied cell style

[TedBrookings]

What changes are being made? (What feature/bug is being fixed here?) -->
Fixes #956. When cell style is explicitly provided for a header line, use the cell style instead of the header style.

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

I'm not sure how to go about writing a test for this change, or even if its necessary given the small scope.

[gmischler]

I'm not sure how to go about writing a test for this change, or even if its necessary given the small scope.

Yes, you want a test for this. It's really quite simple: Create an example that applies a style to the heading row and another one to an individual header cell within it. In the old version the cell style would get overwritten by the general header style. In the new version either the cell style will take precedence or they will get merged (see above). That will demonstrate the changed behaviour of the new code, and catch any regressions that might happen in the future.

Also, you want to describe the new behaviour in "docs/Tables.md".

[TedBrookings]

I believe I've responded to your comments. I had some trouble running some of the tests, likely because I'm working on an m1 mac. For example my new test fails because the object ID and creation date fail, but I see that those are supposed to be removed from diffs.

Because I took your suggestion to add a combine function for FontFace, I added a test for that function. And I made the test for heading styling override a bit more complex to demonstrate combinations of override features.

[Lucas-C]

For example my new test fails because the object ID and creation date fail, but I see that those are supposed to be removed from diffs.

Have you used assert_pdf_equal() to generate your reference PDF files?
cf. https://py-pdf.github.io/fpdf2/Development.html#assert_pdf_equal-writing-new-tests

[TedBrookings]

No, I didn't notice the generate keyword previously. So I had just run the same commands and used pdf.output. That fixed the problems. I'm going to clean up a few things and re-submit. Thanks so much for your help!

[gmischler]

Nice work. Solid and clean implementation, clear documentation, and useful test case.

Can't even find any details to nitpick over... 😉

You'll just have to rebase and resolve the conflict in the changelog, and then it can be merged.

[TedBrookings]

Thanks, I've fixed the issue with the test pdf, and resolved the changelog conflict. Should be all set.

[gmischler]

Did you run the complete test suite locally?
The "table_capture_font_settings" test is failing, and others might as well.
Please check if there is any visible difference in the output. If so, then you'll have to fix that, otherwise you can replace the affected files.

[TedBrookings]

Sorry about that, it's fixed now. It was relying for its behavior on the heading styles not being overridden by supplied cell styles.
I have run the complete test suite, unfortunately I have not been able to get the tests involving ghostscript to succeed despite trying several workarounds, so there are always a bunch that fail, and I didn't notice this new one. It's one of the problems of developing on an M1 mac.

[TedBrookings]

I see it failed in test_load_image::test_shared_images_cache. That passed locally for me:

test/image/test_load_image.py::test_load_text_file PASSED                                                                                                       [ 44%]
test/image/test_load_image.py::test_load_base64_data PASSED                                                                                                     [ 44%]
test/image/test_load_image.py::test_load_invalid_base64_data PASSED                                                                                             [ 44%]
test/image/test_load_image.py::test_share_images_cache PASSED 

Is there something I should do on my end?

[gmischler]

The shared images test has some timing checks built in, which may fail randomly.

But there are also some real issues: Please run pylint and black on the fpdf and test directories before committing, and fix anything they might complain about.

[codecov-commenter]

Codecov Report

Attention: 4 lines in your changes are missing coverage. Please review.

Comparison is base (d81018d) 93.79% compared to head (25d48fe) 93.75%.

❗ Current head 25d48fe differs from pull request most recent head 2a9736c. Consider uploading reports for the commit 2a9736c to get more accurate results

Additional details and impacted files

@@            Coverage Diff             @@
##           master     #979      +/-   ##
==========================================
- Coverage   93.79%   93.75%   -0.04%     
==========================================
  Files          28       28              
  Lines        8396     8410      +14     
  Branches     1545     1549       +4     
==========================================
+ Hits         7875     7885      +10     
- Misses        323      325       +2     
- Partials      198      200       +2     

Files	Coverage Δ
fpdf/table.py	94.52% <100.00%> (ø)
fpdf/fonts.py	94.84% <71.42%> (-1.38%)	⬇️

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

[gmischler]

Finally there. Thanks for your first code contribution, @TedBrookings !

[TedBrookings]

Thanks for bearing with me while I got up to speed!

[Lucas-C]

@allcontributors please add @TedBrookings for code

[allcontributors]

@Lucas-C

I've put up a pull request to add @TedBrookings! 🎉