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

INTPYTHON-549 Rework the projection logic to allow reading list-of-struct data structures using a specified schema #285

Merged
merged 22 commits into from
Mar 26, 2025

Conversation

zpks
Copy link
Contributor

@zpks zpks commented Mar 14, 2025

see #284

@blink1073
Copy link
Member

Thank you @zpks! Can you please add a test that covers your reported bug to test_arrow.py?

@blink1073
Copy link
Member

It looks like this introduced some other failures. Are you able to run the test suite locally?

@zpks
Copy link
Contributor Author

zpks commented Mar 26, 2025

Hello! Thank you for looking, and my apologies for creating the PR prematurely

After some wrangling I was able to test locally.
The test for the desired feature works, it should display what's added.
I initially tried using the already-existing _create_nested_data() function with nested_elem=True, however that functionality does not work.

The change I made changes the projections from {"struct_col": {"field_1": True, "field_2": True}} to {"struct_col.field_1": True, "struct_col.field_2": True} - this works better with the mongo driver, as mongo interperts the dot as separating struct and field - and this also works for list-of-struct structures.
This meant I also had to edit the tests for the projections in order of those to run.
I have edited the tests for the projections - as those change, because in the previous format they errored out in the mongo read.

There was still an unrelated test failing TestExplicitPolarsApi.test_exceptions_for_unsupported_polar_types
I have not touched that code, so was unsure on how to fix those

@zpks
Copy link
Contributor Author

zpks commented Mar 26, 2025

interestingly enough my local ruff ran fine, but the github version finds a too-long line

@blink1073
Copy link
Member

There was still an unrelated test failing TestExplicitPolarsApi.test_exceptions_for_unsupported_polar_types
I have not touched that code, so was unsure on how to fix those

If you merge from master that should be fixed by #288

@blink1073
Copy link
Member

We have instructions for running the linters locally, or in this case you can copy-paste the affected lines from the diff.

https://github.com/mongodb-labs/mongo-arrow/blob/main/bindings/python/CONTRIBUTING.md#running-linters

@zpks
Copy link
Contributor Author

zpks commented Mar 26, 2025

I installed and initialized pre-commit, it ran all it's steps and passed - but I'll run it with tox too

@zpks
Copy link
Contributor Author

zpks commented Mar 26, 2025

Okay I rebased on main (I had to sync my fork first - again, my apology for the mess) and explicityly ran the linter, that fixed the issue - I suppose I had only installed pre-commit after the too-long line was committed. Thank you for your help!

@blink1073 blink1073 changed the title Rework the projection logic to allow reading list-of-struct data structures using a specified schema INTPYTHON-549 Rework the projection logic to allow reading list-of-struct data structures using a specified schema Mar 26, 2025
Copy link
Member

@blink1073 blink1073 left a comment

Choose a reason for hiding this comment

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

Thanks very much!

@blink1073 blink1073 merged commit 7d7f233 into mongodb-labs:main Mar 26, 2025
44 checks passed
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