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

podio_class_generator: only write files if their content changed #65

Merged
merged 1 commit into from
Feb 17, 2020

Conversation

andresailer
Copy link
Member

Avoids spurious re-compilation
BEGINRELEASENOTES

  • podio_class_generator: only write files if their content changed

ENDRELEASENOTES

Copy link
Contributor

@vvolkl vvolkl left a comment

Choose a reason for hiding this comment

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

Great, the recompilation issue was indeed an annoyance when using podio.

Comment on lines +1049 to +1051
existing_content = open(fullname, 'r').read()
if existing_content != content:
open(fullname, 'w').write(content)
Copy link
Contributor

Choose a reason for hiding this comment

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

These lines could be slightly more "pythonic" with a with statement:

with open(fullname, "r+") as _f:
  existing_content = _f.read()
  if existing_content != content:
    _f.write(content)

Copy link
Member Author

Choose a reason for hiding this comment

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

#!/usr/bin/env python
content ='foo bar'
fullname = 'temp.txt'
with open(fullname, "r+") as _f:
  existing_content = _f.read()
  if existing_content != content:
    _f.write(content)

Errors if temp.txt does not exist

$ python testRead.py
Traceback (most recent call last):
  File "testRead.py", line 6, in <module>
    with open(fullname, "r+") as _f:
IOError: [Errno 2] No such file or directory: 'temp.txt'

So we would still need one path for exists, one without.

Copy link
Contributor

Choose a reason for hiding this comment

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

That's true, I overlooked that in one case the file won't exist yet. Taking that in consideration I think the code is best as it is!

Copy link
Member Author

Choose a reason for hiding this comment

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

There are many things to be cleaned up, especially in view of python3 compatibility (#66)

@gaede gaede merged commit a646f68 into AIDASoft:master Feb 17, 2020
@andresailer andresailer deleted the onlyWriteNew branch February 17, 2020 16:12
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.

3 participants