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

[refactor] Use the csv library to generate properly escaped TSV files #116

Closed
wants to merge 4 commits into from
Closed

[refactor] Use the csv library to generate properly escaped TSV files #116

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
c2d96e8
Add more robust routines for BOM file output
aakatz3 Jul 20, 2020
a634493
Fix todo in file read
aakatz3 Jul 21, 2020
5b8b043
Update README.md
aakatz3 Jul 21, 2020
6b3a89e
Addressed review comments
aakatz3 Jul 24, 2020
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
3 changes: 2 additions & 1 deletion README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -99,10 +99,11 @@ $ wireviz ~/path/to/file/mywire.yml
This will output the following files

```
mywire.gv GraphViz output
mywire.gv Raw GraphViz DOT file output of wiring diagram
mywire.svg Wiring diagram as vector image
mywire.png Wiring diagram as raster image
mywire.bom.tsv BOM (bill of materials) as tab-separated text file
mywire.bom.csv BOM (bill of materials) as comma-separated, excel-format text file
mywire.html HTML page with wiring diagram and BOM embedded
```

Expand Down
8 changes: 4 additions & 4 deletions src/wireviz/Harness.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,9 +3,9 @@

from wireviz.DataClasses import Connector, Cable
from graphviz import Graph
from wireviz import wv_colors, wv_helper
from wireviz import wv_colors, wv_helper, bom_helper
from wireviz.wv_colors import get_color_hex
from wireviz.wv_helper import awg_equiv, mm2_equiv, tuplelist2tsv, \
from wireviz.wv_helper import awg_equiv, mm2_equiv, \
nested_html_table, flatten2d, index_if_list, html_line_breaks, \
graphviz_line_breaks, remove_line_breaks, open_file_read, open_file_write
from collections import Counter
Expand Down Expand Up @@ -297,8 +297,8 @@ def output(self, filename: (str, Path), view: bool = False, cleanup: bool = True
graph.save(filename=f'{filename}.gv')
# bom output
bom_list = self.bom_list()
with open_file_write(f'{filename}.bom.tsv') as file:
file.write(tuplelist2tsv(bom_list))
# todo: support user choices of BOM format (probably also graphviz outputs, html outputs)
bom_helper.generate_bom_outputs(filename,bom_list,bom_helper.WIREVIZ_TSV, bom_helper.EXCEL_CSV)
Copy link
Collaborator

Choose a reason for hiding this comment

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

See comment in bom_helper.py.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Understood, leaving comment for future reference, unless requested otherwise.

# HTML output
with open_file_write(f'{filename}.html') as file:
file.write('<!DOCTYPE html>\n')
Expand Down
42 changes: 42 additions & 0 deletions src/wireviz/bom_helper.py
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,42 @@
#!/usr/bin/env python3
# -*- coding: utf-8 -*-

import csv
from wireviz import wv_helper
from wireviz.wv_helper import open_file_write

EXCEL_CSV = csv.excel
EXCEL_TSV = csv.excel_tab
UNIX_CSV = csv.unix_dialect
WIREVIZ_TSV = type('Wireviz BOM', (csv.Dialect, object), dict(
delimiter='\t',
doublequote=True,
escapechar=None,
lineterminator='\n',
quoting=0,
skipinitialspace=False,
strict=False,
quotechar='"'
))
csv.register_dialect('Wireviz BOM', WIREVIZ_TSV)

_csv_formats = { EXCEL_CSV, UNIX_CSV }
_tsv_formats = { EXCEL_TSV, WIREVIZ_TSV }

_csv_ext = '.bom.csv'
_tsv_ext = '.bom.tsv'

def generate_bom_outputs(base_filename, bomdata, *argv):
expanded_csv_names = len(_csv_formats.intersection(set(argv))) > 1
expanded_tsv_names = len(_tsv_formats.intersection(set(argv))) > 1
for fmt in argv:
if fmt in _csv_formats:
file = csv.writer(open_file_write(base_filename + ("_" + fmt.__name__ if expanded_csv_names else "") + _csv_ext, fmt.lineterminator), fmt)

elif fmt in _tsv_formats:
file = csv.writer(open_file_write(base_filename + ("_"+fmt.__name__ if expanded_tsv_names else "") + _tsv_ext, fmt.lineterminator), fmt)
else:
raise KeyError("Unknown BOM Format Specified")
file.writerows(wv_helper.flatten2d(bomdata))
Copy link
Collaborator

Choose a reason for hiding this comment

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

The *argv parsing (lines 29 to 31) are not very pythonic, I'd request you change it.
The len(intersection(set())) > 1 is rather cryptic... and the use case of calling this function requesting multiple CSV and TSV formats at once is pretty much an edge case IMHO... especially since currently there are set defaults, inaccessible to the user.

My proposal: Call the function with a list as the third argument:

def generate_bom_outputs(base_filename, bomdata, formats):
    if not isinstance(formats, List):
        formats = [formats]

and then iterate over the list.
It could then be called using

bom_helper.generate_bom_outputs(filename,bom_list,[bom_helper.WIREVIZ_TSV, bom_helper.EXCEL_CSV])

Copy link
Contributor Author

Choose a reason for hiding this comment

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

That could work well! I will also check briefly to see if the Excel TSV format is essentially compatible with the one wireviz natively generates (and probably have the OS choose if new lines are unix or windows style), and then the csv formats can be directly passed through.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Also, excuse any python style issues, I will try to fix them whenever pointed out. I'm a Java / C / MATLAB developer, primarily, who has wanted to learn python for far too long!

Copy link
Collaborator

Choose a reason for hiding this comment

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

I'm a Java / C / MATLAB developer, primarily, who has wanted to learn python for far too long!

You're not alone, I've done mostly embedded programming in C++ / Arduino before.
I'm loving Python's readability and trying to keep WireViz as pythonic and understandable as possible :)


# TODO: Possibly refactor other BOM output operations, such as HTML, into here?
Copy link
Collaborator

Choose a reason for hiding this comment

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

Yes, but in a separate PR :)

Copy link
Collaborator

Choose a reason for hiding this comment

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

See the discussion in #123

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Understood, leaving comment for future reference, unless requested otherwise.

5 changes: 3 additions & 2 deletions src/wireviz/build_examples.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -74,10 +74,11 @@ def build_tutorials():

file.write(f'![](tutorial{outfile_name}.png)\n\n')

file.write(f'[Bill of Materials](tutorial{outfile_name}.bom.tsv)\n\n\n')
file.write(f'[Bill of Materials - TSV](tutorial{outfile_name}.bom.tsv)\n\n')
file.write(f'[Bill of Materials - CSV](tutorial{outfile_name}.bom.csv)\n\n\n')

def clean_examples():
generated_extensions = ['.gv', '.png', '.svg', '.html', '.bom.tsv']
generated_extensions = ['.gv', '.png', '.svg', '.html', '.bom.tsv', '.bom.csv']

for filepath in [examples_path, demos_path, tutorials_path]:
print(filepath)
Expand Down
6 changes: 3 additions & 3 deletions src/wireviz/wv_helper.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -113,8 +113,8 @@ def remove_line_breaks(inp):
return inp.replace('\n', ' ').rstrip() if isinstance(inp, str) else inp

def open_file_read(filename):
# TODO: Intelligently determine encoding
# TODO: Intelligently determine encoding (UnicodeDammit, Chardet, cchardet are not very reliable in testing)
aakatz3 marked this conversation as resolved.
Show resolved Hide resolved
return open(filename, 'r', encoding='UTF-8')

def open_file_write(filename):
return open(filename, 'w', encoding='UTF-8')
def open_file_write(filename, newline='\n'):
return open(filename, 'w', encoding='UTF-8', newline=newline)