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

feature/add utils get records #1378

Merged
merged 4 commits into from
Jan 20, 2024

Conversation

lavigne958
Copy link
Collaborator

@lavigne958 lavigne958 commented Jan 19, 2024

  • improve docstring - add small example
  • Add migration guide for method get_all_records

closes #1376
closes #1367
closes #976

@lavigne958 lavigne958 self-assigned this Jan 19, 2024
@lavigne958 lavigne958 force-pushed the feature/add_utils_get_records branch from 4c1aea3 to 7ddfd50 Compare January 19, 2024 20:56
Add the new section in the v6 migration guide about
the method `Worksheet.get_all_records` to allow users
to migrate from previous method signature to the new one.

closes #1376

Signed-off-by: Alexandre Lavigne <[email protected]>
Add a new util function that helps building record values.
Records are list of dictionaries, each dictionary has the same
set of keys, they are set from the given list of headers.
Each key is associated to a value from the given matrix of values.
They are as many dictionaries as they are lines in the matrix.

Each line in the matrix will be associated to a dictionary,
each key is associated to a value from that line in the given order.

Example:
Headers: A B C
Values:
1 2 3
7 8 9
Result: [ { A: 1, B: 2, C: 3}, {A: 7, B: 8, C: 9}]

closes #1367

Signed-off-by: Alexandre Lavigne <[email protected]>
@lavigne958 lavigne958 force-pushed the feature/add_utils_get_records branch from 7ddfd50 to de307cb Compare January 19, 2024 20:59
@lavigne958
Copy link
Collaborator Author

thought I could re-open the PR but no, I had to rebase it on tarrget branch then cherry-pick the original commit 😉

ready for review with:

  • migration guide
  • extra docstring with examples
  • new function utils.to_records
  • use of the new function for get_all_records

@lavigne958 lavigne958 requested a review from alifeee January 19, 2024 21:00
@lavigne958 lavigne958 added this to the 6.0.0 milestone Jan 20, 2024
Copy link
Collaborator

@alifeee alifeee left a comment

Choose a reason for hiding this comment

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

this looks good to me :)

I briefened the migration guide to fit in with the rest of it.

@lavigne958 lavigne958 merged commit 406e0a1 into feature/release_6_0_0 Jan 20, 2024
10 checks passed
@lavigne958 lavigne958 deleted the feature/add_utils_get_records branch January 20, 2024 18:47
@alifeee alifeee mentioned this pull request Jan 26, 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.

2 participants