-
Notifications
You must be signed in to change notification settings - Fork 1.2k
I copied the code from the documentation and adjusted it to make it run on my computer #35
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
base: master
Are you sure you want to change the base?
Conversation
Not sure what this is supposed to add to the example. I only see a list of dictionaries being used as a list of dictionaries, which doesn't have much to do with python-docx. I'd suggest we close this unless I'm missing something. |
Yes, I'm not sure I would demonstrate it quite this way, but I think it's a legitimate complaint about the example. Every so often we get someone mentioning that the example doesn't work as stated. It's invariably a beginner, but it seems reasonable to me we should have an example that works when cut and pasted, just so someone starting out can get some immediate gratification :) |
So what can I do here? Is there anything to fix or will you merge? This is a really old thread... |
Hello drm, sorry about my last message I must have been too tired to see through. You did fix issues within the example code (which apparently is not used to generate the rendered image of the example). You shouldn't have removed the Inches class import though, as it is used for setting the picture width. I generated the demo.docx file, but only have LibreOffice to visualize the result (which differs a bit -- for example the table's style seems to not have been applied). It'd be nice that the image be generated at the time we build the documentation, but for now I'd propose that dmr's change be integrated and this issue closed (with the exception of the Inches import line removal). |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Only change requested: leave the imports as they were.
@@ -18,10 +18,8 @@ Here's an example of what |docx| can do: | |||
|img| :: | |||
|
|||
from docx import Document | |||
from docx.shared import Inches |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Even though it works without it (I'm not sure how?), we use Inches in the example. I'd suggest we leave it there, because "explicit is better than implicit".
@@ -46,11 +44,16 @@ Here's an example of what |docx| can do: | |||
hdr_cells[0].text = 'Qty' | |||
hdr_cells[1].text = 'Id' | |||
hdr_cells[2].text = 'Desc' | |||
recordset = [ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good! Not sure where recordset came from before originally.
Corrected the import |
LGTM, except there's apparently a merge conflict. |
There was no recordset in the example on https://python-docx.readthedocs.org/en/latest/ so I created it. Do you want to include it?