Skip to content

remove deprecated stringcase dependency#114

Merged
dmosorast merged 7 commits into
masterfrom
TDL-26784/remove-stringcase
Jan 9, 2025
Merged

remove deprecated stringcase dependency#114
dmosorast merged 7 commits into
masterfrom
TDL-26784/remove-stringcase

Conversation

@leslievandemark
Copy link
Copy Markdown
Contributor

@leslievandemark leslievandemark commented Jan 7, 2025

Description of change

removes the stringcase library and replaces used functions.

Manual QA steps

  • ran the tap locally.

Risks

  • The stringcase pascalcase function is buggy for some edge cases. We opted to create a new implementation to transform strings into pascalcase. Where pascalcase function is used in the code, it is always transforming a snakecase stream name into a pascalcase stream name. If a stream name changed to something weird, the old implementation in stringcase might have transformed it differently than the new one. However, the stream names are hard coded so I don't see this being an issue.

Rollback steps

  • revert this branch

AI generated code

https://internal.qlik.dev/general/ways-of-working/code-reviews/#guidelines-for-ai-generated-code

  • this PR has been written with the help of GitHub Copilot or another generative AI tool

Comment thread tap_bing_ads/__init__.py Outdated
return (string[0]).lower() + re.sub(r"[A-Z]", lambda matched: '_' + (matched.group(0)).lower(), string[1:])

def pascalcase(string):
string = snakecase(string)
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Two passes over string seems a little wasteful.

If you add a new function to do the word splitting, then you can reuse it in snakecase and pascalcase.

def make_words(string):
    pass

def snakecase(string):
    return "_".join(word.lower() for word in make_words(string))

def pascalcase(string):
    return "".join(word.title() for word in make_words(string))

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I was trying to keep all the functionality of the old library, which gets a little tricky with one pass over splitting. However, the use case in the tap is only for CamelCase hardcoded stream names to snake_case and back. I'll update the function to something simpler that fits that case.

@dmosorast dmosorast merged commit 355244d into master Jan 9, 2025
@dsprayberry dsprayberry deleted the TDL-26784/remove-stringcase branch July 17, 2025 18:19
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