Update ToSnakeCase so it wouldn't return "image_u_r_l" for "imageURL"

[hvuhsg]

fixes #4531

Currently any filename and variable name that is converted to snake case get a separator (default "_") before every uppercase letter excluding some edge cases.

So having the filename "NotrixSDK" will become "notrix_s_d_k" and not the desired "notrix_sdk".

This PR is fixing it by checking if the previous letter is upper case and in this case it does not add the separator.

[hvuhsg]

@microsoft-github-policy-service agree

[baywet]

Thanks for the contribution! Can you please also add an entry in the changelog (unreleased, changed)

[hvuhsg]

Updated changelog

[andrueastman]

@hvuhsg Thanks for the contribution.

Any chance you can double check the failing tests as well?

[hvuhsg]

I checked the tests, It seems to have many dependencies for this format "4XX" -> "4_x_x" and I do not have the expertise or the time to update them.

I will not be continuing this PR I hop you could take it from here.

[baywet]

@andrueastman it seems the current change would impact the way namespaces are generated, and subsequently the imports.
this "from .error4_x_x import Error4XX" would probably need to be updated to that "from .error4XX import Error4XX" in all the python writer tests.
It wouldn't impact Graph for the error scenario, but might impact it for other scenarios, so we should kick off generation for the SDK before we merge this PR in.
@hvuhsg understood, thanks for the contributions! We'll need to run validation before changing anything else on this PR.

[andrueastman]

@baywet Looking at the diff here https://github.com/microsoftgraph/msgraph-sdk-python/compare/main..v1.0/pipelinebuild/147301

We're going to change

• namespacing (e.g. models.o_data_errors.o_data_error to models.odata_errors.odata_error)
• File names (e.g ios_vpp_ebook.py to ios_vpp_e_book.py)
• Property names (e.g minimum_cpu_speed_in_mhz to minimum_cpu_speed_in_m_hz)

It may probably make sense to change this to an overload and limit this to variable name scenarios and track this for 2.0 change similar to #2495.

[baywet]

@andrueastman Thanks for validating this!
Can you provide more information about the overload option and how it'd be used in a v1?

[andrueastman]

The idea would be essentially to be

1. Add a boolean parameter(default false) to the ToSnakeCase method to enable/disable the uppercase filter
2. Update caller instances to pass true for scenarios of parameter names and possibly file names (will need confirmation on breaking behavior) renaming so as to keep other scenarios as is (other languages can also choose to opt-in once evaluated).
3. Create issue to remove parameter in for v2 milestone so that the behaviour is changed when we move to v2.

[baywet]

@andrueastman the named argument syntax makes changing the parameter names a breaking change foo(pos=10, forcenamed=20)
As per the file name, I think it'd would impact the imports, in which case this would represent a breaking change as well.

my suggestion at this point would be to leave this PR open in draft until we start working on a v2