jazzband / tablib

Python Module for Tabular Datasets in XLS, CSV, JSON, YAML, &c.
https://tablib.readthedocs.io/
MIT License
4.6k stars 592 forks source link

Get row method #557

Closed yoonthegoon closed 1 year ago

yoonthegoon commented 1 year ago

Fixes #24 get_row, get_col methods

yoonthegoon commented 1 year ago
=================================== FAILURES ===================================
___________________________ TablibTestCase.test_get ____________________________
Traceback (most recent call last):
  File "/Users/runner/hostedtoolcache/Python/3.9.17/x64/lib/python3.9/unittest/case.py", line 59, in testPartExecutor
    yield
  File "/Users/runner/hostedtoolcache/Python/3.9.17/x64/lib/python3.9/unittest/case.py", line 592, in run
    self._callTestMethod(testMethod)
  File "/Users/runner/hostedtoolcache/Python/3.9.17/x64/lib/python3.9/unittest/case.py", line 550, in _callTestMethod
    method()
  File "/Users/runner/work/tablib/tablib/tests/test_tablib.py", line 209, in test_get
    self.assertRaises(self.founders.get(3), IndexError)
  File "/Users/runner/work/tablib/tablib/.tox/py/lib/python3.9/site-packages/tablib/core.py", line 507, in get
    return self[index]
  File "/Users/runner/work/tablib/tablib/.tox/py/lib/python3.9/site-packages/tablib/core.py", line 173, in __getitem__
    _results = self._data[key]
IndexError: list index out of range
    def test_get(self):
        """Verify getting rows by index"""

        self.assertEqual(self.founders.get(0), self.john)
        self.assertEqual(self.founders.get(1), self.george)
        self.assertEqual(self.founders.get(2), self.tom)

        self.assertEqual(self.founders.get(-1), self.tom)
        self.assertEqual(self.founders.get(-2), self.george)
        self.assertEqual(self.founders.get(-3), self.john)

        self.assertRaises(self.founders.get(3), IndexError)  # <- 🤦

Let me fix this real quick. Also forgot to build the docs, so getting that done too.

codecov[bot] commented 1 year ago

Codecov Report

Merging #557 (3b36bfc) into master (f3ef2e9) will increase coverage by 0.26%. Report is 3 commits behind head on master. The diff coverage is 100.00%.

@@            Coverage Diff             @@
##           master     #557      +/-   ##
==========================================
+ Coverage   91.37%   91.64%   +0.26%     
==========================================
  Files          28       28              
  Lines        2713     2800      +87     
==========================================
+ Hits         2479     2566      +87     
  Misses        234      234              
Files Changed Coverage Δ
src/tablib/core.py 85.15% <100.00%> (+0.13%) :arrow_up:
src/tablib/formats/_html.py 100.00% <100.00%> (ø)
tests/test_tablib.py 98.77% <100.00%> (+0.04%) :arrow_up:

... and 2 files with indirect coverage changes

:mega: We’re building smart automated test selection to slash your CI/CD build times. Learn more

yoonthegoon commented 1 year ago

Would one of you please take a look when you get the chance? I'm looking to try to clean out some old issues on here when I get time. @hugovk @claudep

hugovk commented 1 year ago

Thanks for the PR.

https://github.com/jazzband/tablib/pull/28 added get_col, should this be named get_row instead of get (as suggested in https://github.com/jazzband/tablib/issues/24)?

yoonthegoon commented 1 year ago

Thanks for the PR.

https://github.com/jazzband/tablib/pull/28 added get_col, should this be named get_row instead of get (as suggested in https://github.com/jazzband/tablib/issues/24)?

i was originally gonna, but all the row methods in the class don't have row in their names while all the column methods do have col. kenneth also commented on the original issue that the methods he wanted were get to return a row by index and get_col for column by index.

claudep commented 1 year ago

Did you check if the docs should be updated somewhere?

yoonthegoon commented 1 year ago

I built the docs locally and it looks like it added it by me, so the existing rst files appear to pick it up. docs/_build/* is in the .gitignore so I don't think there's any need for me to commit a build. Built docs are here and look good to me.

hugovk commented 1 year ago

We could add an example to the quickstart guide:

https://tablib.readthedocs.io/en/stable/tutorial.html#selecting-rows-columns

yoonthegoon commented 1 year ago

Realized that the get method simply just calls self.__getitem__ which can either be an index or header acting as either a slice or dict. I made sure that get only takes int as having a get row method work like

>>> data.get(0)
('Kenneth', 'Reitz', 22)
>>> data.get('First Name')
['Kenneth', 'Bessie']

would be inconsistent and confusing.

claudep commented 1 year ago

The "add type validation" commit should not be part of this PR.

yoonthegoon commented 1 year ago

Also accidentally ran black on fixing a flake8 issue running tox 🙃

yoonthegoon commented 1 year ago

The "add type validation" commit should not be part of this PR.

Yeah I just noticed it changed more than I intended. Lemme fix that real quick

claudep commented 1 year ago

:medal_sports: