Skip to content

Fix bug when field gets converted to blacklisted word#36

Merged
bogdandm merged 1 commit intobogdandm:masterfrom
a1d4r:fix_blacklist
Jun 8, 2021
Merged

Fix bug when field gets converted to blacklisted word#36
bogdandm merged 1 commit intobogdandm:masterfrom
a1d4r:fix_blacklist

Conversation

@a1d4r
Copy link
Copy Markdown
Contributor

@a1d4r a1d4r commented Jun 7, 2021

I found out that if a field name contains upper-case letters, then the underscore won't be appended to the resulting field name if it is reserved (blacklisted, in terms of the project). For example, one might expect ID to be converted to id_ but it appears as id in the model.

The solution is to change the order of string operations:

  1. Convert unicode
  2. Get rid of non-word characters
  3. Make sure the field name starts with a letter
  4. Convert to camel case
  5. Check if the field name is blacklisted

@coveralls
Copy link
Copy Markdown

Coverage Status

Coverage decreased (-0.06%) to 98.375% when pulling 2b7d331 on a1d4r:fix_blacklist into 62d77e6 on bogdandm:master.

@bogdandm
Copy link
Copy Markdown
Owner

bogdandm commented Jun 8, 2021

Thank you, this is pretty good fix. I think there is one more minor case when after all conversions class name could shadow other global definitions in camel case (like True/False/Exception). But it could be fixed sometime in the future, it's quite rare to see field named liked this. I will look closely to this later.

@bogdandm bogdandm merged commit a528def into bogdandm:master Jun 8, 2021
@bogdandm
Copy link
Copy Markdown
Owner

bogdandm commented Jun 8, 2021

Currently I could not publish new minor version because of broken/outdated Travic-CI pipeline 😞
https://travis-ci.org/github/bogdandm/json2python-models/jobs/773948039

I will migrate this project to https://www.travis-ci.com/ (or maybe github-actions) in few weeks and than I will make a new release.

@a1d4r a1d4r deleted the fix_blacklist branch June 8, 2021 18:13
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.

3 participants