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.

Sign up for GitHub

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

Jump to bottom

Fix parsing of csv template files #219

Merged
merged 5 commits into from
Sep 22, 2021
Merged

Fix parsing of csv template files #219

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
71bd53d
Fix parsing of csv template files
gmischler Sep 18, 2021
d70bc37
fixes suggested by static code check
gmischler Sep 18, 2021
6ed9686
Update template.py
gmischler Sep 19, 2021
fa62a8d
now it's dark.
gmischler Sep 20, 2021
f1d7802
do some hardcoded template tests without multiline
gmischler Sep 21, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
25 changes: 14 additions & 11 deletions docs/Templates.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -47,12 +47,13 @@ from fpdf import Template

#this will define the ELEMENTS that will compose the template.
elements = [
{ 'name': 'company_logo', 'type': 'I', 'x1': 20.0, 'y1': 17.0, 'x2': 78.0, 'y2': 30.0, 'font': None, 'size': 0.0, 'bold': 0, 'italic': 0, 'underline': 0, 'foreground': 0, 'background': 0, 'align': 'I', 'text': 'logo', 'priority': 2, },
{ 'name': 'company_name', 'type': 'T', 'x1': 17.0, 'y1': 32.5, 'x2': 115.0, 'y2': 37.5, 'font': 'helvetica', 'size': 12.0, 'bold': 1, 'italic': 0, 'underline': 0, 'foreground': 0, 'background': 0, 'align': 'I', 'text': '', 'priority': 2, },
{ 'name': 'box', 'type': 'B', 'x1': 15.0, 'y1': 15.0, 'x2': 185.0, 'y2': 260.0, 'font': 'helvetica', 'size': 0.0, 'bold': 0, 'italic': 0, 'underline': 0, 'foreground': 0, 'background': 0, 'align': 'I', 'text': None, 'priority': 0, },
{ 'name': 'box_x', 'type': 'B', 'x1': 95.0, 'y1': 15.0, 'x2': 105.0, 'y2': 25.0, 'font': 'helvetica', 'size': 0.0, 'bold': 1, 'italic': 0, 'underline': 0, 'foreground': 0, 'background': 0, 'align': 'I', 'text': None, 'priority': 2, },
{ 'name': 'line1', 'type': 'L', 'x1': 100.0, 'y1': 25.0, 'x2': 100.0, 'y2': 57.0, 'font': 'helvetica', 'size': 0, 'bold': 0, 'italic': 0, 'underline': 0, 'foreground': 0, 'background': 0, 'align': 'I', 'text': None, 'priority': 3, },
{ 'name': 'barcode', 'type': 'BC', 'x1': 20.0, 'y1': 246.5, 'x2': 140.0, 'y2': 254.0, 'font': 'Interleaved 2of5 NT', 'size': 0.75, 'bold': 0, 'italic': 0, 'underline': 0, 'foreground': 0, 'background': 0, 'align': 'I', 'text': '200000000001000159053338016581200810081', 'priority': 3, },
{ 'name': 'company_logo', 'type': 'I', 'x1': 20.0, 'y1': 17.0, 'x2': 78.0, 'y2': 30.0, 'font': None, 'size': 0.0, 'bold': 0, 'italic': 0, 'underline': 0, 'foreground': 0, 'background': 0, 'align': 'I', 'text': 'logo', 'priority': 2, 'multiline': 0},
{ 'name': 'company_name', 'type': 'T', 'x1': 17.0, 'y1': 32.5, 'x2': 115.0, 'y2': 37.5, 'font': 'helvetica', 'size': 12.0, 'bold': 1, 'italic': 0, 'underline': 0, 'foreground': 0, 'background': 0, 'align': 'I', 'text': '', 'priority': 2, 'multiline': 0},
{ 'name': 'multline_text', 'type': 'T', 'x1': 20, 'y1': 100, 'x2': 40, 'y2': 105, 'font': 'helvetica', 'size': 12, 'bold': 0, 'italic': 0, 'underline': 0, 'foreground': 0, 'background': 0x88ff00, 'align': 'I', 'text': 'Lorem ipsum dolor sit amet, consectetur adipisici elit', 'priority': 2, 'multiline': 1}
{ 'name': 'box', 'type': 'B', 'x1': 15.0, 'y1': 15.0, 'x2': 185.0, 'y2': 260.0, 'font': 'helvetica', 'size': 0.0, 'bold': 0, 'italic': 0, 'underline': 0, 'foreground': 0, 'background': 0, 'align': 'I', 'text': None, 'priority': 0, 'multiline': 0},
{ 'name': 'box_x', 'type': 'B', 'x1': 95.0, 'y1': 15.0, 'x2': 105.0, 'y2': 25.0, 'font': 'helvetica', 'size': 0.0, 'bold': 1, 'italic': 0, 'underline': 0, 'foreground': 0, 'background': 0, 'align': 'I', 'text': None, 'priority': 2, 'multiline': 0},
{ 'name': 'line1', 'type': 'L', 'x1': 100.0, 'y1': 25.0, 'x2': 100.0, 'y2': 57.0, 'font': 'helvetica', 'size': 0, 'bold': 0, 'italic': 0, 'underline': 0, 'foreground': 0, 'background': 0, 'align': 'I', 'text': None, 'priority': 3, 'multiline': 0},
{ 'name': 'barcode', 'type': 'BC', 'x1': 20.0, 'y1': 246.5, 'x2': 140.0, 'y2': 254.0, 'font': 'Interleaved 2of5 NT', 'size': 0.75, 'bold': 0, 'italic': 0, 'underline': 0, 'foreground': 0, 'background': 0, 'align': 'I', 'text': '200000000001000159053338016581200810081', 'priority': 3, 'multiline': 0},
]

#here we instantiate the template and define the HEADER
Expand All @@ -75,10 +76,12 @@ See template.py or [Web2Py] (Web2Py.md) for a complete example.
You define your elements in a CSV file "mycsvfile.csv"
that will look like:
```
line0;T;20.0;13.0;190.0;13.0;times;10.0;0;0;0;0;65535;C;;0
line1;T;20.0;67.0;190.0;67.0;times;10.0;0;0;0;0;65535;C;;0
name0;T;21;14;104;25;times;16.0;0;0;0;0;0;C;;2
title0;T;64;26;104;30;times;10.0;0;0;0;0;0;C;;2
line0;L;20.0;12.0;190.0;12.0;times;0.5;0;0;0;0;16777215;C;;0;0
line1;L;20.0;36.0;190.0;36.0;times;0.5;0;0;0;0;16777215;C;;0;0
name0;T;21.0;14.0;104.0;25.0;times;16.0;0;0;0;0;16777215;L;name;2;0
title0;T;21.0;26.0;104.0;30.0;times;10.0;0;0;0;0;16777215;L;title;2;0
multiline;T;21.0;50.0;28.0;54.0;times;10.5;0;0;0;0;16777215;L;multi line;0;1
numeric_text;T;21.0;80.0;100.0;84.0;times;10.5;0;0;0;0;16777215;R;007;0;0
```

Remember that each line represents an element and each field represents one of the properties of the element in the following order:
Expand All @@ -92,7 +95,7 @@ def test_template():
title="Sample Invoice")
f.parse_csv("mycsvfile.csv", delimiter=";")
f.add_page()
f["company_name"] = "Sample Company"
f["name0"] = "Joe Doe"
return f.render("./template.pdf")

```
51 changes: 30 additions & 21 deletions fpdf/template.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -74,26 +74,37 @@ def load_elements(self, elements):
self.elements = elements
self.keys = [v["name"].lower() for v in self.elements]

def _parse_colorcode(self, s):
""" Allow hex and oct values for colors """
s = s.strip()
if not s:
raise ValueError('Foreground and Background must be numeric')
Copy link
Collaborator

Choose a reason for hiding this comment

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

maybe consider putting a different error message as s is "not empty" here, not "not numeric". (unless i am missing something?)

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I phrased it in this way to inform the user about what's supposed to go there. "Not empty" doesn't give that information.
The documentation could also be more specific about this, but fleshing that out is a project for another time...

if s[:2] in ['0x', '0X']:
return int(s, 16)
elif s[0] == '0':
return int(s, 8)
return int(s)

def parse_csv(self, infile, delimiter=",", decimal_sep=".", encoding=None):
"""Parse template format csv file and create elements dict"""
keys = (
"name",
"type",
"x1",
"y1",
"x2",
"y2",
"font",
"size",
"bold",
"italic",
"underline",
"foreground",
"background",
"align",
"text",
"priority",
"multiline",
handlers = (
("name", str.strip),
("type", str.strip),
("x1", float),
Copy link
Collaborator

Choose a reason for hiding this comment

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

i like this approach. "in the name of deterministic behavior" why strip the fields? but this is a good place to put the actual conversions, and less lines of code than what i was considering when i put the util function in.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Everything got stripped in the original version, so I just left it that way in order not to change the behaviour. The alternative would be to just use str as a NOP.

("y1", float),
("x2", float),
("y2", float),
("font", str.strip),
("size", float),
("bold", int),
("italic", int),
("underline", int),
("foreground", self._parse_colorcode),
("background", self._parse_colorcode),
("align", str.strip),
("text", str.strip),
("priority", int),
("multiline", int),
)
self.elements = []
self.pg_no = 0
Expand All @@ -105,9 +116,7 @@ def parse_csv(self, infile, delimiter=",", decimal_sep=".", encoding=None):
for i, v in enumerate(row):
if not v.startswith("'") and decimal_sep != ".":
v = v.replace(decimal_sep, ".")
stripped_value = v.strip()
typed_value = try_to_type(stripped_value)
kargs[keys[i]] = typed_value
kargs[handlers[i][0]] = handlers[i][1](v)
self.elements.append(kargs)
self.keys = [v["name"].lower() for v in self.elements]

Expand Down
10 changes: 6 additions & 4 deletions test/template/mycsvfile.csv
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,6 @@
line0;L;20.0;12.0;190.0;12.0;times;0.5;0;0;0;0;16777215;C;;0
line1;L;20.0;36.0;190.0;36.0;times;0.5;0;0;0;0;16777215;C;;0
name0;T;21;14;104;25;times;16.0;0;0;0;0;16777215;L;name;2
title0;T;21;26;104;30;times;10.0;0;0;0;0;16777215;L;title;2
line0;L;20.0;12.0;190.0;12.0;times;0.5;0;0;0;0;16777215;C;;0;0
line1;L;20.0;36.0;190.0;36.0;times;0.5;0;0;0;0;16777215;C;;0;0
name0;T;21.0;14.0;104.0;25.0;times;16.0;0;0;0;0;16777215;L;name;2;0
title0;T;21.0;26.0;104.0;30.0;times;10.0;0;0;0;0;16777215;L;title;2;0
multiline;T;21.0;50.0;28.0;54.0;times;10.5;0;0;0;0;0xffff00;L;multi line;0;1
numeric_text;T;21.0;80.0;100.0;84.0;times;10.5;0;0;0;0;16777215;R;007;0;0
Binary file modified test/template/template_nominal_csv.pdf
View file
Open in desktop
Binary file not shown.
Binary file modified test/template/template_nominal_hardcoded.pdf
View file
Open in desktop
Binary file not shown.
28 changes: 27 additions & 1 deletion test/template/test_template.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ def test_template_nominal_hardcoded(tmp_path):
"align": "I",
"text": "logo",
"priority": 2,
"multiline": 0,
Copy link
Member

Choose a reason for hiding this comment

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

Did multiline become mandatory?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I only changed the tests in this area. Previously multiline was not tested at all, so I added it there.
But if it's not mandatory (obviously, since it passed before), I guess both cases should be tested, so I'll remove some of them agein.
Btw.: Are ther any other fields that are optional? The documentation only mentions a default in passing, but doesn't explicitly say which are or aren't mandatory.

},
{
"name": "company_name",
Expand All @@ -45,6 +46,26 @@ def test_template_nominal_hardcoded(tmp_path):
"align": "I",
"text": "",
"priority": 2,
"multiline": 0,
},
{
"name": "multline_text",
"type": "T",
"x1": 20,
"y1": 100,
"x2": 40,
"y2": 105,
"font": "helvetica",
"size": 12,
"bold": 0,
"italic": 0,
"underline": 0,
"foreground": 0,
"background": 0x88ff00,
"align": "I",
"text": "Lorem ipsum dolor sit amet, consectetur adipisici elit",
"priority": 2,
"multiline": 1,
},
{
"name": "box",
Expand All @@ -62,6 +83,7 @@ def test_template_nominal_hardcoded(tmp_path):
"align": "I",
"text": None,
"priority": 0,
"multiline": 0,
},
{
"name": "box_x",
Expand All @@ -79,6 +101,7 @@ def test_template_nominal_hardcoded(tmp_path):
"align": "I",
"text": None,
"priority": 2,
"multiline": 0,
},
{
"name": "line1",
Expand All @@ -96,6 +119,7 @@ def test_template_nominal_hardcoded(tmp_path):
"align": "I",
"text": None,
"priority": 3,
"multiline": 0,
},
{
"name": "barcode",
Expand All @@ -113,6 +137,7 @@ def test_template_nominal_hardcoded(tmp_path):
"align": "I",
"text": "200000000001000159053338016581200810081",
"priority": 3,
"multiline": 0,
},
]
tmpl = Template(format="A4", elements=elements, title="Sample Invoice")
Expand All @@ -124,7 +149,8 @@ def test_template_nominal_hardcoded(tmp_path):


def test_template_nominal_csv(tmp_path):
"""Taken from docs/Templates.md"""
"""Same data as in docs/Templates.md
The numeric_text tests for a regression."""
tmpl = Template(format="A4", title="Sample Invoice")
tmpl.parse_csv(HERE / "mycsvfile.csv", delimiter=";")
tmpl.add_page()
Expand Down