Add command to export log files to CSV #557
Conversation
rafaelsteil
force-pushed
the
export-csv-pr
branch
4 times, most recently
from
61d2914
to
757712a
Compare
src/plotman/csv_exporter.py
Outdated
| def send_to_stdout(logfilenames): | ||
| generate(logfilenames, sys.stdout) | ||
|
|
||
| def header(writer): |
There was a problem hiding this comment.
This is a pretty long list to just carefully keep aligned between the two places that the columns are and values are identified. https://github.com/ericaltendorf/plotman/pull/299/files#diff-62b25901762e697ab64f8088b67b720707e694198e9d26ce3fafd3d08b14006fR262-R325 or such maybe but let's take a look again when the other files are here.
There was a problem hiding this comment.
Not sure if I understood. Mind to clarify?
There was a problem hiding this comment.
There are 34 items here and also 34 items specified around line 80. It would be easy to get these out of order, or add a new item one place and not the other. There would be no exception, I don't think, just misaligned column headings and data. The linked code certainly isn't the only solution. It was just one that I had coded recently (at that point). Looking here again now I see that many of the fields are calculated from others. This makes the linked code less obviously good.
Another thought, have a function that can create a dict from a PlotInfo object, maybe a method on PlotInfo, and use the keys from that as the header. There is also a .writeheader() method. Then when you pass a dict to be written it will make sure the data goes in the matching column.
This PR depends on #532 |
rafaelsteil
force-pushed
the
export-csv-pr
branch
from
757712a
to
04bbe88
Compare
src/plotman/csv_exporter.py
Outdated
| writer.writerow([ | ||
| info.plot_id, | ||
| info.started_at, | ||
| parse_date(info.started_at).strftime('%Y-%m-%d'), |
There was a problem hiding this comment.
iso without time
src/plotman/csv_exporter.py
Outdated
| def send_to_stdout(logfilenames): | ||
| generate(logfilenames, sys.stdout) | ||
|
|
||
| def header(writer): |
There was a problem hiding this comment.
There are 34 items here and also 34 items specified around line 80. It would be easy to get these out of order, or add a new item one place and not the other. There would be no exception, I don't think, just misaligned column headings and data. The linked code certainly isn't the only solution. It was just one that I had coded recently (at that point). Looking here again now I see that many of the fields are calculated from others. This makes the linked code less obviously good.
Another thought, have a function that can create a dict from a PlotInfo object, maybe a method on PlotInfo, and use the keys from that as the header. There is also a .writeheader() method. Then when you pass a dict to be written it will make sure the data goes in the matching column.
src/plotman/csv_exporter.py
Outdated
| save_to_file(logfilenames, save_to) | ||
|
|
||
| def save_to_file(logfilenames, filename: str): | ||
| with open(filename, 'w') as file: |
There was a problem hiding this comment.
https://docs.python.org/3/library/csv.html#examples
If I remember correctly, this relates to quoted fields with newlines in them. Unlikely, but best to just always open files as the CSV modules suggests. (detail, py2 is different...)
| with open(filename, 'w') as file: | |
| with open(filename, 'w', newline='') as file: |
src/plotman/csv_exporter.py
Outdated
| from plotman.log_parser import PlotLogParser | ||
| from plotman.plotinfo import PlotInfo | ||
|
|
||
| def export(logfilenames, save_to = None): |
There was a problem hiding this comment.
I would probably tend to leave the file opening to the caller instead of having these small 'helpers'. It just generally seems better to leave the file handling explicit at the higher levels.
|
(Discussed the comments on Keybase) |
This PR depends on #532
Adds a command to export all plot logs as CSV. It can be to STDOUT (default) or to a specific file via the flag
-o <filename>