HDFStore.select_column error reporting #20705
Conversation
gfyoung
added
Error Reporting
pandas/io/pytables.py
Outdated
| @@ -1094,7 +1093,8 @@ def get_storer(self, key): | |||
| """ return the storer object for a key, raise if not in the file """ | |||
| group = self.get_node(key) | |||
| if group is None: | |||
| return None | |||
| raise KeyError('No object named %s in the file' % key) | |||
|
|
|||
There was a problem hiding this comment.
Can we use the .format syntax here.
There was a problem hiding this comment.
Sure, no problems.
pandas/tests/io/test_pytables.py
Outdated
| # GH 17912 | ||
| # HDFStore.select_column should raise a KeyError | ||
| # exception if the key is not a valid store | ||
| pytest.raises(KeyError, store.select_column, 'df', 'index') |
There was a problem hiding this comment.
Can we also check for the error message here?
pandas/io/pytables.py
Outdated
| @@ -887,7 +887,9 @@ def remove(self, key, where=None, start=None, stop=None): | |||
| where = _ensure_term(where, scope_level=1) | |||
| try: | |||
| s = self.get_storer(key) | |||
| except: | |||
| except KeyError as ke: | |||
There was a problem hiding this comment.
Can this just be
except KeyError:
raiseMaybe w/ a comment explaining what's going on. Or is that python 3 only?
There was a problem hiding this comment.
@TomAugspurger : The syntax is supported in Python 2 and 3. This would be a good simplification.
There was a problem hiding this comment.
I've just checked the Python 2 docs, and it seems to be compatible with that version as well (https://docs.python.org/2.7/tutorial/errors.html). If you prefer this style, I'll change the code and add the comment.
There was a problem hiding this comment.
This is pretty standard syntax IMO (we use in other places in other codebase), so I'm not sure how necessary commenting is. That being said, couldn't hurt, so let's just comment it :)
CianciuStyles
force-pushed
the
fix-issue-17912
branch
from
9a474ff
to
03b41be
Compare
|
Hello @CianciuStyles! Thanks for updating the PR. Cheers ! There are no PEP8 issues in this Pull Request. 🍻 Comment last updated on April 15, 2018 at 21:00 Hours UTC |
Codecov Report
@@ Coverage Diff @@
## master #20705 +/- ##
==========================================
- Coverage 91.84% 91.84% -0.01%
==========================================
Files 153 153
Lines 49279 49279
==========================================
- Hits 45259 45258 -1
- Misses 4020 4021 +1
Continue to review full report at Codecov.
|
…lumn when the key is not a valid store
CianciuStyles
force-pushed
the
fix-issue-17912
branch
from
03b41be
to
a9d4c6a
Compare
|
The latest push should address all the comments so far. |
|
thanks @CianciuStyles nice patch! |
git diff upstream/master -u -- "*.py" | flake8 --diff