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

Extend load_file to handle file objects. #379

Merged
merged 7 commits into from
Jul 28, 2021
Merged

Extend load_file to handle file objects. #379

merged 7 commits into from
Jul 28, 2021

Conversation

niklassiemer
Copy link
Member

No description provided.

@niklassiemer niklassiemer added the enhancement New feature or request label Jul 23, 2021
@niklassiemer niklassiemer added the integration Start the integration tests with pyiron_atomistics/contrib for this PR label Jul 27, 2021
@niklassiemer niklassiemer requested a review from pmrv July 27, 2021 10:04
Comment on lines 88 to 92
if is_file:
with open(file, encoding='utf8') as f:
return f.readlines()
else:
return file.readlines()
Copy link
Contributor

Choose a reason for hiding this comment

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

I'm not sure the extra flag is needed, maybe something like

Suggested change
if is_file:
with open(file, encoding='utf8') as f:
return f.readlines()
else:
return file.readlines()
if instance(file, io.IOBase):
return file.readlines()
else:
with open(file, encoding='utf8') as f:
return _load_txt(f)

is better.

Copy link
Member Author

Choose a reason for hiding this comment

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

Probably we should then go the whole way and use this check instead of generating the filename_is_string at all.

return _load_default(fp, is_file=filename_is_str)


class FileDataTemplate(ABC):
Copy link
Contributor

Choose a reason for hiding this comment

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

I didn't see it before, but where else is the FileDataTemplate supposed to be used?

Copy link
Member Author

Choose a reason for hiding this comment

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

I hope to have it as a Base class for storing File related data. I am using this also for the S3FileData here.

@niklassiemer niklassiemer merged commit f8c085d into master Jul 28, 2021
@delete-merged-branch delete-merged-branch bot deleted the load_file branch July 28, 2021 13:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request integration Start the integration tests with pyiron_atomistics/contrib for this PR
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants