Skip to content

Conversation

arokem
Copy link
Member

@arokem arokem commented Jun 2, 2016

Currently, when a file doesn't exist (for example, if you typo'd the file-name), you get a relatively cryptic error: "couldn't work out file-type for file foo.nii.gz". I propose that we check if the file exists first, and raise an informative error up front if it doesn't exist.

arokem added 2 commits June 2, 2016 11:33
Currently raises a "couldn't work out file-type for this file" error.
Raises an informative error when the file doesn't exist.
@matthew-brett
Copy link
Member

Seems reasonable. Only possible problem would be if the extra check added significant time, for example on slower NFS systems.

@matthew-brett
Copy link
Member

FileNotFoundError seems to be Python 3 only.

@arokem
Copy link
Member Author

arokem commented Jun 3, 2016

Bummer. I switched it to IOError instead (works on both 2.7 and 3.5 on my laptop). There are other hacks that can be done (along these lines: http://stackoverflow.com/a/21368622/3532933), but that seems like too much here.

@coveralls
Copy link

Coverage Status

Coverage increased (+0.001%) to 95.851% when pulling 65e8932 on arokem:file-not-found into 7e41691 on nipy:master.

@matthew-brett
Copy link
Member

Believe it or not - I hate to be annoying - but it would be nice to be able trap the file not found error specifically - maybe add it to nibabel/py3k.py as class FileNotFoundError(IOError) for Python 2.

@codecov-io
Copy link

codecov-io commented Jun 7, 2016

Current coverage is 93.78%

Merging #455 into master will increase coverage by <.01%

@@             master       #455   diff @@
==========================================
  Files           147        147          
  Lines         19181      19186     +5   
  Methods           0          0          
  Messages          0          0          
  Branches       2029       2030     +1   
==========================================
+ Hits          17988      17993     +5   
  Misses          796        796          
  Partials        397        397          

Powered by Codecov. Last updated by 7e41691...65e8932

@arokem
Copy link
Member Author

arokem commented Jun 7, 2016

Not annoying at all. Seems like a good idea. Did I get it right?

On Mon, Jun 6, 2016 at 2:56 PM, Matthew Brett [email protected]
wrote:

Believe it or not - I hate to be annoying - but it would be nice to be
able trap the file not found error specifically - maybe add it to
nibabel/py3k.py as class FileNotFoundError(IOError) for Python 2.


You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub
#455 (comment), or mute
the thread
https://github.com/notifications/unsubscribe/AAHPNqLHWQOIDHERKfvekUJ-xkh0Be10ks5qJJebgaJpZM4Is2g0
.

@coveralls
Copy link

Coverage Status

Coverage decreased (-0.003%) to 95.847% when pulling 366fa0e on arokem:file-not-found into 7e41691 on nipy:master.

@matthew-brett
Copy link
Member

Looks OK to me - just a PEP8 nitpick on travis - otherwise good to go.

@arokem
Copy link
Member Author

arokem commented Jun 7, 2016

Cool. Added the necessary white-space.

On Tue, Jun 7, 2016 at 12:14 AM, Matthew Brett [email protected]
wrote:

Looks OK to me - just a PEP8 nitpick on travis - otherwise good to go.


You are receiving this because you commented.
Reply to this email directly, view it on GitHub
#455 (comment), or mute
the thread
https://github.com/notifications/unsubscribe/AAHPNvAc74dzifl0t5Fi5tB7HzFHWF_bks5qJRpWgaJpZM4Is2g0
.

@matthew-brett
Copy link
Member

Thanks for doing that - merging.

@matthew-brett matthew-brett merged commit 312bc4d into nipy:master Jun 7, 2016
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.

4 participants