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

Add access by column #1002

Closed
wants to merge 6 commits into from
Closed

Add access by column #1002

wants to merge 6 commits into from

Conversation

laowantong
Copy link

@laowantong laowantong commented Mar 27, 2024

Describe your changes

The proposed method ResultSet.column() iterates on a column given by name.

def column(self, name):
"""Iterator yielding the values on a column specified by name"""
try:
column_index = list(self.keys).index(name)
except ValueError:
raise KeyError(f"Column '{name}' not found in the result set")
for row in self:
yield row[column_index]

Additions:

  • a method column(name) in resultset.py (above);
  • a method test_resultset_column(result_set) in test_resultset.py with a successful test and a failed test;
  • a paragraph in doc_intro.md;
  • an entry in changelog.md.

Checklist before requesting a review


📚 Documentation preview 📚: https://jupysql--1002.org.readthedocs.build/en/1002/

@neelasha23
Copy link

@laowantong Hey is there a related issue? Please mention the issue number also

@laowantong
Copy link
Author

No existing issue. I have just tried to add a feature which I felt was missing for my usage. Not sure why some checks fail. Feel free to close this PR is the procedure is not OK or you think the feature is useless.

@neelasha23
Copy link

No existing issue. I have just tried to add a feature which I felt was missing for my usage. Not sure why some checks fail. Feel free to close this PR is the procedure is not OK or you think the feature is useless.

Sure will have a look. There are lint issues in the changes. Please refer to contributing guidelines
Also, do include a description of the changes in the PR description @laowantong

@laowantong
Copy link
Author

Sorry for spamming you with my failed attempts to make the checks pass. There are still lint issues, although pkgfmt succeeds on my machine:

$ pkgmt format
Running command:black .
All done! ✨ 🍰 ✨
115 files left unchanged.
Running command:nbqa black .
All done! ✨ 🍰 ✨
56 files left unchanged.
Finished formatting with black!

Just reformat the files I have actually touched, since the other ones seem to block the PR
@neelasha23
Copy link

Sorry for spamming you with my failed attempts to make the checks pass. There are still lint issues, although pkgfmt succeeds on my machine:

The lints are passing but the testcase test_resultset_column is failing. @laowantong

@laowantong
Copy link
Author

Too much friction for me, sorry. I cannot run the tests on my machine. There are multiple failures everywhere. I give up. You can close this PR. Thanks.

@edublancas edublancas closed this Mar 30, 2024
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.

3 participants