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

Fixes bug in string case serialization not taking numbers into account #420

Merged
merged 9 commits into from
Jun 25, 2023

Conversation

matt035343
Copy link
Collaborator

As described in #376, the camelCase serialization does not remove underscores in front of numbers. I believe it should - let me know what you think.

@matt035343 matt035343 linked an issue Jun 10, 2023 that may be closed by this pull request
Copy link
Collaborator

@george-zubrienko george-zubrienko left a comment

Choose a reason for hiding this comment

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

You can run into issues with this when having fields named a_1_1 vs a_11 - both will have the same name in Camel. I think underscores should be replaced by any a-Z char instead OR you should check if camel will produce duplicate fields when applied, and runtime fail the serialisation.

@matt035343
Copy link
Collaborator Author

You can run into issues with this when having fields named a_1_1 vs a_11 - both will have the same name in Camel. I think underscores should be replaced by any a-Z char instead OR you should check if camel will produce duplicate fields when applied, and runtime fail the serialisation.

imo, I think adding arbitrary chars is worse than underscores as it may also cause collisions with the original name. Then I think it is a better to raise an exception. Also, it is possible to provide custom field names in the metadata to manually avoid the conflicts.

@george-zubrienko
Copy link
Collaborator

a better to raise an exception

Agreed :)

@george-zubrienko
Copy link
Collaborator

@matt035343 any chance you have time to add exception in case duplicate is produced and we merge? :)

@matt035343
Copy link
Collaborator Author

@matt035343 any chance you have time to add exception in case duplicate is produced and we merge? :)

Exception is now added, please take a look

Copy link
Collaborator

@george-zubrienko george-zubrienko left a comment

Choose a reason for hiding this comment

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

Awesome!

@matt035343 matt035343 merged commit 21d2ab2 into master Jun 25, 2023
@matt035343 matt035343 deleted the string-case-number-bug-fix branch June 25, 2023 12:53
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.

Leading underscore are not kept in to_dict method
2 participants