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

DOGE-55: Fix dict snake case dump #1409

Closed
wants to merge 7 commits into from

Conversation

ddonukis
Copy link
Contributor

@ddonukis ddonukis commented Oct 6, 2023

Description

Addressing issue: #1087

Checklist:

  • Tests added/updated.
  • Documentation updated. Documentation is generated from docstrings - these must be updated according to your change.
    If a new method has been added it should be referenced in cognite.rst in order to generate docs based on its docstring.
  • Changelog updated in CHANGELOG.md.
  • Version bumped. If triggering a new release is desired, bump the version number in _version.py and pyproject.toml per semantic versioning.

@codecov
Copy link

codecov bot commented Oct 6, 2023

Codecov Report

Merging #1409 (5ffde64) into master (57f8eb8) will decrease coverage by 0.02%.
The diff coverage is 87.50%.

@@            Coverage Diff             @@
##           master    #1409      +/-   ##
==========================================
- Coverage   90.65%   90.63%   -0.02%     
==========================================
  Files         119      119              
  Lines       14357    14361       +4     
==========================================
+ Hits        13015    13016       +1     
- Misses       1342     1345       +3     
Files Coverage Δ
cognite/client/data_classes/labels.py 95.08% <100.00%> (ø)
cognite/client/data_classes/shared.py 94.91% <ø> (ø)
cognite/client/utils/_text.py 100.00% <100.00%> (ø)
cognite/client/data_classes/extractionpipelines.py 84.39% <50.00%> (ø)

... and 3 files with indirect coverage changes

if camel_case:
return {to_camel_case(key): value for key, value in dct.items()}
else:
return {to_snake_case(key): value for key, value in dct.items()}
Copy link
Contributor

Choose a reason for hiding this comment

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

Can’t you use convert_dict_to_case?

@haakonvt
Copy link
Contributor

@ddonukis this is still in draft? Or is it ready for review?

@ddonukis
Copy link
Contributor Author

This needs more work.

@haakonvt
Copy link
Contributor

haakonvt commented Nov 1, 2023

Closing as v7 will fix this by removing dict subclasses altogether 👌 #1452

@haakonvt haakonvt closed this Nov 1, 2023
@haakonvt haakonvt deleted the fix-DOGE-55-dict-snake-case-dump branch November 1, 2023 12:12
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.

2 participants