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

Update to reportlab 4 #708

Merged
merged 1 commit into from Oct 1, 2023
Merged

Update to reportlab 4 #708

merged 1 commit into from Oct 1, 2023

Conversation

timobrembeck
Copy link
Collaborator

Short description

Update to reportlab 4

Proposed changes

  • Remove version restriction
  • Document installation of C extensions

Resolved issues

Fixes: #699

@timobrembeck
Copy link
Collaborator Author

@stefan6419846 do you think this will do?

@stefan6419846
Copy link
Contributor

Apart from the fact that this will break reportlab 3 due to the requirements and enforce the (recommended, but not yet officially required) Cairo backend, it looks okay for me. To avoid this, we probably would have to utilize requirement extras groups, which I am not sure of how much additional maintenance effort this would introduce.

- Remove version restriction
- Document installation of C extensions
@timobrembeck
Copy link
Collaborator Author

Apart from the fact that this will break reportlab 3 due to the requirements and enforce the (recommended, but not yet officially required) Cairo backend, it looks okay for me. To avoid this, we probably would have to utilize requirement extras groups, which I am not sure of how much additional maintenance effort this would introduce.

Hmm yes, I agree that it's not ideal. But for now probably better than the previous restriction. At least people who rely on old versions of reportlab can just pin xhtml2pdf to an older version as well...
In the long term, we can evaluate the use of extra dependencies, probably after #692 has been merged...

@timobrembeck timobrembeck merged commit 5a3bb7e into master Oct 1, 2023
5 checks passed
@timobrembeck timobrembeck deleted the reportlab branch October 1, 2023 19:31
@viniciusd
Copy link

viniciusd commented Nov 8, 2023

I am still debugging it, so I am sharing in case someone else had the same issue with the new version, I have reportlab==4.0.7 and xhtml2pdf==0.2.12.

I am getting the exception:

    pdf = pisa.pisaDocument(BytesIO(html.encode("UTF-8")), dest=result, encoding='UTF-8')
  File xhtml2pdf/document.py, line 196, in pisaDocument
    doc.build(context.story)
  File reportlab/platypus/doctemplate.py, line 1082, in build
    self.handle_flowable(flowables)
  File reportlab/platypus/doctemplate.py, line 931, in handle_flowable
    if frame.add(f, canv, trySplit=self.allowSplitting):
  File reportlab/platypus/frames.py, line 169, in _add
    w, h = flowable.wrap(aW, h)
  File xhtml2pdf/xhtml2pdf_reportlab.py, line 832, in wrap
    remainingWidth -= colWidth
TypeError: unsupported operand type(s) for -=: 'float' and 'str'

remainingWidth is the float number, colWidth here is the string "12%".

I will do some digging before stating it is an issue/bug.

@viniciusd
Copy link

viniciusd commented Nov 8, 2023

Yeah, it is a bug caused by a refactoring when the ruff linter was added:
a3211b8#diff-74f01775643512741726dc5b4de3062a970be3ce0a07bd148192173000fb872dL822

I will create a separate issue for that.

Edit
It was already described in #731

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.

Reportlab 4 compatibility
3 participants