Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Deprecating txt= arg in favor of text= - solves #853 #903

Merged
merged 1 commit into from
Oct 11, 2023

Conversation

Lucas-C
Copy link
Member

@Lucas-C Lucas-C commented Aug 25, 2023

This MR aims to harmonize argument naming.
Before this MR:

  • FPDF.cell(), FPDF.multi_cell(), FPDF.text() & FPDF.write() accepted a txt= argument
  • FPDF.write_html() & table.Row.cell() accepted a text= argument

After merging this PR:

  • all public fpdf2 methods that accepted txt= will now also accept text=
  • text= will be the documented & recommended way to pass this argument
  • passing txt= will still work but generate a DeprecationWarning

The decorator idea came from a suggestion by @gmischler there: #897 (comment)

⚠️ This PR should probably be merged just before releasing, to avoid having our online documentation being updated with text mentioned in method docstrings, whereas they do no support this argument in v2.7.5

@Lucas-C Lucas-C requested a review from gmischler as a code owner August 25, 2023 06:58
@Lucas-C Lucas-C mentioned this pull request Aug 25, 2023
5 tasks
@Lucas-C Lucas-C changed the title Deprecating txt= arg in favor of text= Deprecating txt= arg in favor of text= - solves #853 Aug 25, 2023
Copy link
Collaborator

@gmischler gmischler left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looking good!

You're probably correct about merging just before release. Although given the sweeping nature of the change, interference with other PRs is likely but hard to avoid.

fpdf/deprecation.py Show resolved Hide resolved
fpdf/html.py Show resolved Hide resolved
@Lucas-C Lucas-C force-pushed the deprecated_txt_to_text_arg branch 3 times, most recently from 62f2ec1 to aec15d2 Compare October 11, 2023 06:59
@Lucas-C Lucas-C force-pushed the deprecated_txt_to_text_arg branch from aec15d2 to e0123f1 Compare October 11, 2023 07:32
@codecov-commenter
Copy link

Codecov Report

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

Comparison is base (e918119) 93.63% compared to head (7d5f6a6) 93.61%.

❗ Current head 7d5f6a6 differs from pull request most recent head e0123f1. Consider uploading reports for the commit e0123f1 to get more accurate results

Additional details and impacted files
@@            Coverage Diff             @@
##           master     #903      +/-   ##
==========================================
- Coverage   93.63%   93.61%   -0.02%     
==========================================
  Files          28       28              
  Lines        8211     8227      +16     
  Branches     1501     1503       +2     
==========================================
+ Hits         7688     7702      +14     
- Misses        324      325       +1     
- Partials      199      200       +1     
Files Coverage Δ
fpdf/html.py 95.89% <100.00%> (ø)
fpdf/table.py 94.24% <ø> (ø)
fpdf/template.py 90.39% <100.00%> (ø)
fpdf/fpdf.py 92.74% <97.87%> (+0.01%) ⬆️
fpdf/deprecation.py 93.02% <83.33%> (-3.76%) ⬇️

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

@andersonhc andersonhc merged commit 0d55f2d into master Oct 11, 2023
10 checks passed
@gmischler gmischler mentioned this pull request Oct 31, 2023
5 tasks
@Lucas-C Lucas-C deleted the deprecated_txt_to_text_arg branch December 4, 2023 15:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants