Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Get row method #557

Merged
merged 6 commits into from
Aug 2, 2023
Merged

Get row method #557

merged 6 commits into from
Aug 2, 2023

Conversation

yoonthegoon
Copy link
Contributor

Fixes #24 get_row, get_col methods

@yoonthegoon
Copy link
Contributor Author

yoonthegoon commented Jul 19, 2023

=================================== 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
Copy link

codecov bot commented Jul 19, 2023

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%) ⬆️
src/tablib/formats/_html.py 100.00% <100.00%> (ø)
tests/test_tablib.py 98.77% <100.00%> (+0.04%) ⬆️

... and 2 files with indirect coverage changes

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

@yoonthegoon
Copy link
Contributor Author

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
Copy link
Member

hugovk commented Aug 1, 2023

Thanks for the PR.

#28 added get_col, should this be named get_row instead of get (as suggested in #24)?

@hugovk hugovk added the feature label Aug 1, 2023
@yoonthegoon
Copy link
Contributor Author

Thanks for the PR.

#28 added get_col, should this be named get_row instead of get (as suggested in #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.

Copy link
Member

@hugovk hugovk left a comment

Choose a reason for hiding this comment

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

Ah yes, good point!

Thank you!

@claudep
Copy link
Contributor

claudep commented Aug 2, 2023

Did you check if the docs should be updated somewhere?

@yoonthegoon
Copy link
Contributor Author

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
Copy link
Member

hugovk commented Aug 2, 2023

We could add an example to the quickstart guide:

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

@yoonthegoon
Copy link
Contributor Author

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
Copy link
Contributor

claudep commented Aug 2, 2023

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

@yoonthegoon
Copy link
Contributor Author

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

@yoonthegoon
Copy link
Contributor Author

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 claudep merged commit 98b8c53 into jazzband:master Aug 2, 2023
19 checks passed
@claudep
Copy link
Contributor

claudep commented Aug 2, 2023

🏅

@yoonthegoon yoonthegoon deleted the get_row-method branch August 2, 2023 15:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

get_row, get_col methods
3 participants