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 test to ensure dict with enum keys are encoded properly #1001

Merged

Conversation

adeelsohailahmed
Copy link
Contributor

@adeelsohailahmed adeelsohailahmed commented Aug 15, 2024

The issue raised in #829 seems to have been fixed in PR #868

if isinstance(obj, Mapping):
return {
key if isinstance(key, Enum) else str(key): self.encode(value)
for key, value in obj.items()
}

Only the relevant test was missing to ensure the regression was adequately addressed. This PR adds that test.

07pepa
07pepa previously approved these changes Aug 15, 2024
@adeelsohailahmed adeelsohailahmed requested a review from a team August 15, 2024 22:49
@adeelsohailahmed adeelsohailahmed changed the title test: ensures dict with enum keys are encoded properly Add test to ensure dict with enum keys are encoded properly Aug 16, 2024
Riverfount
Riverfount previously approved these changes Aug 19, 2024
Copy link

@Riverfount Riverfount left a comment

Choose a reason for hiding this comment

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

I think that a good work

@07pepa 07pepa requested a review from Riverfount August 27, 2024 19:55
Copy link

@Riverfount Riverfount left a comment

Choose a reason for hiding this comment

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

LGTM

@07pepa 07pepa marked this pull request as draft September 3, 2024 13:03
@07pepa
Copy link
Member

07pepa commented Sep 3, 2024

raised as issue to pydantic

@adeelsohailahmed adeelsohailahmed marked this pull request as ready for review September 3, 2024 21:10
@adeelsohailahmed
Copy link
Contributor Author

It's good that you're raising this issue in Pydantic, @07pepa. I appreciate that.

This PR, however, is not adding any specific encoding logic to overcome what may be possible Pydantic limitations. It's only adding a test to confirm that the existing Beanie behaviour that had regressed in between 1.23.6 -> 1.24.0 has been fixed.

@07pepa
Copy link
Member

07pepa commented Sep 4, 2024

actualy this is mistage i was too fast and converted this to draft and comented by mystake lets merge it

Copy link
Contributor

@CAPITAINMARVEL CAPITAINMARVEL left a comment

Choose a reason for hiding this comment

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

Looks good

@07pepa
Copy link
Member

07pepa commented Oct 1, 2024

@adeelsohailahmed resolve conficts please

@adeelsohailahmed
Copy link
Contributor Author

@adeelsohailahmed resolve conficts please

I don't see any conflicts.

@adeelsohailahmed adeelsohailahmed merged commit 8cb5aa8 into BeanieODM:main Oct 1, 2024
19 checks passed
@adeelsohailahmed
Copy link
Contributor Author

Thank you for the reviews

@adeelsohailahmed adeelsohailahmed deleted the test/dict-with-enum-keys branch October 1, 2024 15:31
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