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 util function to_records to build records #1377

Merged
merged 1 commit into from
Jan 19, 2024

Conversation

lavigne958
Copy link
Collaborator

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

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 requested a review from alifeee January 15, 2024 23:15
@lavigne958 lavigne958 self-assigned this Jan 15, 2024
@alifeee alifeee changed the title Add util function to_record to build records Add util function to_records to build records Jan 17, 2024
@alifeee
Copy link
Collaborator

alifeee commented Jan 17, 2024

this is good stuff, thank you :)

we should also make this function more visible to users - it should be added to the docstring of get_all_records

(and migration guide readme - covered by #1376)

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.

see above comment

@lavigne958
Copy link
Collaborator Author

this is good stuff, thank you :)

Thanks ! It's actually shorter than what I expected 🙄

we should also make this function more visible to users - it should be added to the docstring of get_all_records

(and migration guide readme - covered by #137)

I did not know if we want to make it into a single PR.

I'll add it to the migration guide, and I'll add a mention about it in the doc string of get_all_records.

What do you think of the outcome of that global change ?
Are you satisfied with it ?

@alifeee
Copy link
Collaborator

alifeee commented Jan 17, 2024

Thanks ! It's actually shorter than what I expected 🙄

haha.. yes! a great one-liner eh

What do you think of the outcome of that global change ?
Are you satisfied with it ?

Do you mean the simplification of get_all_records? I think I am happy with it. I will have to have another look. What do you think?

@lavigne958
Copy link
Collaborator Author

What do you think of the outcome of that global change ?
Are you satisfied with it ?

Do you mean the simplification of get_all_records? I think I am happy with it. I will have to have another look. What do you think?

yes that's what I mean. you can once everything is merged, it might be easier to see the whole result.

I'm working currently on the docstring upgrade + migration guide (that part is quite long to explain, new functions, removed methods etc)

@alifeee alifeee merged commit d7c110f into feature/release_6_0_0 Jan 19, 2024
@alifeee alifeee deleted the feature/add_utils_get_records branch January 19, 2024 16:05
@alifeee alifeee restored the feature/add_utils_get_records branch January 19, 2024 16:26
alifeee added a commit that referenced this pull request Jan 19, 2024
…ecords"

This reverts commit d7c110f, reversing
changes made to 63c4c5e.
@alifeee
Copy link
Collaborator

alifeee commented Jan 19, 2024

my apologies for merging this, I have restored the branch and reverted the merge commit with 51f9f25.

You may need to create a new PR :/

@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