Skip to content

Empty file handing#173

Merged
bjlittle merged 1 commit intoSciTools:masterfrom
bblay:mtfile
Feb 6, 2013
Merged

Empty file handing#173
bjlittle merged 1 commit intoSciTools:masterfrom
bblay:mtfile

Conversation

@bblay
Copy link
Contributor

@bblay bblay commented Oct 24, 2012

A fix for #101 (replaces #165).

@rhattersley
Copy link
Member

I'm not entirely convinced this qualifies as a fix to "Avoid obscure crash...". Invoking iris.load('obs.pp') on an empty file results in a large nine level traceback which finishes with:

  File ".../iris/io/format_picker.py", line 242, in _read_n
    raise iris.exceptions.TranslationError("Unexpected end of file: '{}'".format(filename))
iris.exceptions.TranslationError: Unexpected end of file: 'obs.pp'

Not exactly friendly. Someone unfamiliar with the code won't know the significance of the error:

  • it's emitted from deep in the call stack;
  • and the message doesn't clearly explain why the error occurred.

If we have to emit an error, could we emit it from iris.load instead?

Also, there's no test. ;-)

@ghost ghost assigned pp-mo Dec 7, 2012
@bblay
Copy link
Contributor Author

bblay commented Jan 14, 2013

rebased

@bblay
Copy link
Contributor Author

bblay commented Jan 18, 2013

rebased onto latest master for more thorough testing

Copy link
Member

Choose a reason for hiding this comment

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

Please consider use the temp_filename() context manager for this. It avoids the need to remove the file.

@rhattersley
Copy link
Member

If you're seeing a lot of weird commits in this PR it's because we removed a commit from master and some of the PRs have got themselves confused. A rebase will clear the problem.

NB. It doesn't need a rebase, you can create a new PR with exactly the same commit (i.e. pelson/iris@2bcf972) and it will be just fine. It might be possible to move the head of your PR somewhere else and then back again... but caveat emptor!

@ghost ghost assigned bjlittle Feb 5, 2013
Copy link
Member

Choose a reason for hiding this comment

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

Extra blank line please, while you're there!

@bjlittle
Copy link
Member

bjlittle commented Feb 6, 2013

Could you please squash your commit history, thanks.

@bjlittle
Copy link
Member

bjlittle commented Feb 6, 2013

👍 Lovely work @bblay, it's good to go!

bjlittle added a commit that referenced this pull request Feb 6, 2013
@bjlittle bjlittle merged commit 8923875 into SciTools:master Feb 6, 2013
Copy link
Member

Choose a reason for hiding this comment

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

TranslationError seems a bit strange in this context. I would have said an IOError was better, but it doesn't madly matter now its been merged.

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.

6 participants