graphene icon indicating copy to clipboard operation
graphene copied to clipboard

fix: snake case converter corner cases

Open tcleonard opened this issue 2 years ago • 3 comments

There were some inconsistencies in the way the snake case conversion was working.

  1. the handling of string already containing _ was not consistent. Indeed in the old implementation:
assert to_snake_case("_File__") == "__file__"
assert to_snake_case("_IPhoneHysteria") == "_i_phone_hysteria"  # new implementation "__i_phone_hysteria"
assert to_snake_case("_A0") == "_a0"  # new implementation "__a0"
assert to_snake_case("toto_Ao") == "toto__ao"
assert to_snake_case("toto_A0") == "toto_a0"  # new implementation "toto__a0"
  1. the handling of consecutive capitals was not consistent. Indeed in the old implementation:
assert to_snake_case("SnakesOnAPlane") == "snakes_on_a_plane"
assert to_snake_case("SNakesOnAPlane") == "s_nakes_on_a_plane"
assert to_snake_case("SN0w") == "sn0w"  # new implementation "s_n0w"
assert to_snake_case("SNow") == "s_now"
assert to_snake_case("AAA") == "aaa"  # new implementation "a_a_a"
assert to_snake_case("AAAa") == "aa_aa"  # new implementation "a_a_aa"

tcleonard avatar Aug 08 '23 12:08 tcleonard

Codecov Report

Patch coverage: 100.00% and no project coverage change.

Comparison is base (f5aba2c) 96.01% compared to head (9ee127e) 96.01%. Report is 1 commits behind head on master.

Additional details and impacted files
@@           Coverage Diff           @@
##           master    #1515   +/-   ##
=======================================
  Coverage   96.01%   96.01%           
=======================================
  Files          51       51           
  Lines        1755     1756    +1     
=======================================
+ Hits         1685     1686    +1     
  Misses         70       70           
Files Changed Coverage Δ
graphene/utils/str_converters.py 100.00% <100.00%> (ø)

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

codecov[bot] avatar Aug 08 '23 12:08 codecov[bot]

This is a breaking change, how should we handle this? I believe there should be a feature toggle which we gracefully switch over.

erikwrede avatar Aug 08 '23 13:08 erikwrede

That's fair. When you say "gracefully switch over", what strategy did you have in mind exactly? I doubt the next major version of graphene is around the corner... I'm a bit worried that we won't remember switch this on just before releasing the next major

tcleonard avatar Aug 08 '23 14:08 tcleonard