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

fix: Invert case_sensitive logic in StructType #1147

Merged
merged 4 commits into from
Sep 9, 2024

Conversation

AnthonyLam
Copy link
Contributor

The field_by_name method in StructType seems to have inverted logic on the case_sensitive parameter.

@AnthonyLam AnthonyLam changed the title fix: Invert logic in StructType fix: Invert case_sensitive logic in StructType Sep 8, 2024
@ndrluis
Copy link
Collaborator

ndrluis commented Sep 9, 2024

Hello @AnthonyLam, thank you for your contribution. Could you add a test to ensure the expected behavior?

@AnthonyLam
Copy link
Contributor Author

Hello @AnthonyLam, thank you for your contribution. Could you add a test to ensure the expected behavior?

For sure! I've added a test in test_types.py. Let me know if it's insufficient.

Copy link
Contributor

@kevinjqliu kevinjqliu left a comment

Choose a reason for hiding this comment

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

great catch! thanks for the contribution

@sungwy
Copy link
Collaborator

sungwy commented Sep 9, 2024

Hi @AnthonyLam thank you for working on this PR and @ndrluis , @kevinjqliu thank you for the review! I've triggered the CI, and it looks like there's a few issues with the test. Once those are resolved, I'm happy to do another round of review

@sungwy sungwy merged commit 9b9ed53 into apache:main Sep 9, 2024
8 checks passed
@AnthonyLam AnthonyLam deleted the fix/case_sensitivity branch September 9, 2024 19:40
sungwy pushed a commit to sungwy/iceberg-python that referenced this pull request Dec 7, 2024
* fix: Invert  logic in StructType

* Add test for StructType.field_by_name

* Remove var I forgot about.

* Fix formatting post-lint
sungwy pushed a commit to sungwy/iceberg-python that referenced this pull request Dec 7, 2024
* fix: Invert  logic in StructType

* Add test for StructType.field_by_name

* Remove var I forgot about.

* Fix formatting post-lint
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.

4 participants