Skip to content

BUG: Fix XmpInformation missing method _getText#915

Merged
MartinThoma merged 2 commits intomainfrom
bug-xmp
May 28, 2022
Merged

BUG: Fix XmpInformation missing method _getText#915
MartinThoma merged 2 commits intomainfrom
bug-xmp

Conversation

@MasterOdin
Copy link
Copy Markdown
Member

@MasterOdin MasterOdin commented May 27, 2022

Fixes #914 (though will want to backport)

PR fixes a bug where some methods in the xmp module were referencing the old name of internal functions (_getText) instead of the new version (_get_text).

I suppose this would be an argument to move to only supporting Python 3.7 so that can use forward references for type annotations, so that could do self: XmpInformation throughout these to allow mypy to help catch these sorts of errors.

@codecov
Copy link
Copy Markdown

codecov bot commented May 27, 2022

Codecov Report

Merging #915 (345fb82) into main (2499329) will not change coverage.
The diff coverage is 0.00%.

@@           Coverage Diff           @@
##             main     #915   +/-   ##
=======================================
  Coverage   77.89%   77.89%           
=======================================
  Files          16       16           
  Lines        4324     4324           
  Branches      813      813           
=======================================
  Hits         3368     3368           
  Misses        784      784           
  Partials      172      172           
Impacted Files Coverage Δ
PyPDF2/xmp.py 52.06% <0.00%> (ø)

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 2499329...345fb82. Read the comment docs.

@dzdzdzd123
Copy link
Copy Markdown

Hi, I'm using Github's submission Issues feature for the first time, I was wondering if this bug fix can be fixed until you merge into the main branch and I install the new version again?

@MartinThoma
Copy link
Copy Markdown
Member

Thank you for the fix! I'll merge + fix it on the 1.x branch. I don't understand yet why mypy didn't capture those 🕵️

@MartinThoma MartinThoma merged commit f060edb into main May 28, 2022
@MartinThoma MartinThoma deleted the bug-xmp branch May 28, 2022 08:26
MartinThoma added a commit that referenced this pull request May 28, 2022
Closes #914

Credits to MasterOdin for:
#915
MartinThoma added a commit that referenced this pull request May 28, 2022
@MartinThoma
Copy link
Copy Markdown
Member

MartinThoma commented May 28, 2022

I don't understand yet why mypy didn't capture those

I used Any for self 🤦‍♂️ That explains if, of course.

@MartinThoma
Copy link
Copy Markdown
Member

I was wondering if this bug fix can be fixed until you merge into the main branch and I install the new version again?

Sorry, I don't understand that. Are you asking if there is a way to install from the feature branch? As the fix was already released, it's no longer necessary.

It is possible to install from git (and also from specific branches), but I would need to look the exact syntax/urls up. Might be a good idea to add this to the documentation, although it's rather a pip question than a PyPDF2 question 🤔

@MartinThoma
Copy link
Copy Markdown
Member

MartinThoma commented May 28, 2022

#918 enables mypy / makes this type of mistake impossible (preventing a regression + ensuring we captured all; at least for the PyPDF2 2.0.0 release)

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.

Error in extracting metadata, prompt 'XmpInformation' object has no attribute '_getText'

3 participants