Adding date-time to frames filename #454
Conversation
There was a problem hiding this comment.
The changes LGTM but I believe the style guide prefers "single quotes over double quotes as long as no escaping is necessary".
Thank you for the feedback. Fixed the issue :) |
There was a problem hiding this comment.
Thanks for the PR!
I have a few thoughts about this:
- I think that we should change this to add the date by default. That way if users run it without any arguments, they won't lose data.
- I think we should also provide a
-ooption so that users can control the filename if they really want to have a "stable" filename. - I don't think using
%Y-%m-%d_%H.%Mis actually good enough. What happens if a user runs the same command multiple times in a minute? I think we need to add seconds to this to make it safer.
tf2_tools/tf2_tools/view_frames.py
Outdated
| datetime = time.strftime('%Y-%m-%d_%H.%M.%S') | ||
| frames_gv = '{:s}_{:s}.gv'.format(parsed_args.output, datetime) | ||
| frames_pdf = '{:s}_{:s}.pdf'.format(parsed_args.output, datetime) |
There was a problem hiding this comment.
Almost. If the user specifies a filename, I think that we should honor it exactly without further changes. So this should be:
frames_gv = '{:s}.gv'.format(parsed_args.output)
framed_pdf = '{:s}.pdf'.format(parsed_args.output)
There was a problem hiding this comment.
Ok, I was trying to be fancy with adding a timedstamp to it, if the user runs the same command multiple times with same filename.
There was a problem hiding this comment.
Ok, I was trying to be fancy with adding a timedstamp to it, if the user runs the same command multiple times with same filename.
Hmm, that's a valid concern, but also the user should shoulder such a burden.
There was a problem hiding this comment.
Yeah, agreed. My basic thinking on this is that by default, we should be sure not to overwrite data. But if the user explicitly tells us to overwrite something, we should just go ahead and do it.
|
CI:
|
Working with a problem, creating multiple TF frames to see, what has changed and not noticing to make a copy of the file, overwriting a previous data.
With this, adding -d or --datetime as an argument on the run command, it adds a date-time stamp to the end of the file.
Originally I used this on ros1, then instructed to make for future versions.