Skip to content

Conversation

@JuliaSprenger
Copy link
Member

All IOs implicitly assume that the file and directory names provided are strings. Therefore many IOs fail, when a path specified as a pathlib object is provided instead. This PR introduces the automatic conversion of filenames and directory names, for all IOs and adds a common IO test.

return_path=return_path,
clean=clean)

def create_file_reader(self, filename=None, return_path=False,
Copy link
Contributor

Choose a reason for hiding this comment

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

I don't remember why we add this before.
it is not used anymore ?

Copy link
Member Author

Choose a reason for hiding this comment

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

It is not used within Neo and is also not working, e.g. when trying to create a generic reader as the default filename does not exist.

@samuelgarcia
Copy link
Contributor

Ok for me.

@JuliaSprenger
Copy link
Member Author

I am not sure why circleCI is not receiving output for some time. I think we should investigate this phenomenon in general, since this has been an issue also in other PRs... Maybe it's a matter of adjusting the CI configuration to our current requirements?

@apdavison apdavison added this to the 0.10.0 milestone Mar 4, 2021
@samuelgarcia samuelgarcia self-assigned this Mar 5, 2021
This commits adds a test that is running all IOs with an input
filename / directory name specified as a pathlib object.
@JuliaSprenger
Copy link
Member Author

This is rebased and tests are fixed. @samuelgarcia ready for final review.

@samuelgarcia
Copy link
Contributor

OK for me.
I merge.

@samuelgarcia samuelgarcia merged commit 70aabbc into NeuralEnsemble:master Jun 15, 2021
@JuliaSprenger JuliaSprenger deleted the pathlib_filenames branch April 3, 2023 13:58
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