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

bin/restore-exif-mtime: Handle missing EXIF time #1

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

RichiH
Copy link

@RichiH RichiH commented Jan 1, 2014

Don't simply throw and exception and die; print warning message and
simply continue.

Don't simply throw and exception and die; print warning message and
simply continue.
@RichiH
Copy link
Author

RichiH commented Jan 1, 2014

As an aside, since you are apparently using git-annex as well: https://github.com/RichiH/metamonger

It's Perl, but it's been written precisely for git-annex :)

@glasserc
Copy link
Owner

glasserc commented Jan 1, 2014

Hi, I saw metamonger just the other day. It looks much better suited to do this kind of stuff than my one-off scripts.

I'm not crazy about this patch because:

  • catching except: is almost always wrong from a technical standpoint; you probably want except Exception:. PEP 8 says "A bare except: clause will catch SystemExit and KeyboardInterrupt exceptions, making it harder to interrupt a program with Control-C, and can disguise other problems. If you want to catch all exceptions that signal program errors, use except Exception: (bare except is equivalent to except BaseException:)." (I think this is a flaw in the language, but there you are.)
  • on top of that, catching Exception is very general and could hide any number of (programming) errors. I'd much rather either specify exactly the exception that was raised, or check for the error condition. For example, if realtime is getting None from get_time_exif, check if realtime:. It's true that Python people say "easier to ask forgiveness than permission", but that's more about types and type checking than about error conditions.
  • the "pass" isn't needed here; by definition it's a null statement, it's just useful when some statement is needed syntactically.

@RichiH
Copy link
Author

RichiH commented Jan 2, 2014

Hi,

my current workflow is to use your script to pre-assign mtime for easier sorting and renaming and to then store that info in metamonger. I will stop doing that once metamonger has proper EXIF support.

I will play with my patch a bit; as it's been my first Python code ever... I may have strong-armed my way through ;)

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