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.

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

Improve compatibility and precision of date information in log filenames #174

Merged
merged 3 commits into from
Apr 28, 2021

Conversation

BasilHorowt
Copy link
Collaborator

There a few reasons that the current log filenames may not be ideal:

  1. They contain : which breaks in Windows and is shown as / in OSX GUI (#144)
  2. They don't contain any more precision than seconds, so with many concurrent plots being created (or error situations), naming collisions can occur
  3. They don't contain any timezone information which makes them not fully comprehensive

This potentially closes #144

Note: As discussed on chat with @altendky, we settled on +HH_MM / -HH_MM for the timezone part (±[hh]:[mm] with the same : -> _ done for times). it seems that strftime provides the offset as one component via %z, without the option to use a HH:MM format. For now I have left it without a separator, but it would be trivial to insert a _ if it is desired. Thoughts?

There a few reasons that the current log filenames may not be ideal:

1) They contain `:` which breaks in Windows and is shown as `/` in OSX GUI ([ericaltendorf#144](ericaltendorf#144))
2) They don't contain any more precision than seconds, so with many concurrent plots being created (or error situations), naming collisions can occur
3) They don't contain any timezone information which makes them not fully comprehensive

This potentially closes ericaltendorf#144

Note: As discussed on chat with @altendky, we settled on `+HH_MM` / `-HH_MM` for the timezone part (`±[hh]:[mm]` with the same `:` -> `_` done for times).  it seems that `strftime` provides the offset as one component via `%z`, without the option to use a `HH:MM` format.  For now I have left it without a separator, but it would be trivial to insert a `_` if it is desired. Thoughts?
Copy link
Collaborator

@altendky altendky left a comment

Choose a reason for hiding this comment

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

Maybe use .isoformat(), but otherwise looks good. Thanks.

src/plotman/manager.py Outdated Show resolved Hide resolved
Copy link
Collaborator

@altendky altendky left a comment

Choose a reason for hiding this comment

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

Cool, cool. Thanks for the design and coding work.

@altendky altendky merged commit 06de974 into ericaltendorf:development Apr 28, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants