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

Use jaraco.collections for code re-use on CaseInsensitiveDict. #1654

Merged
merged 4 commits into from Dec 17, 2017

Conversation

Projects
None yet
3 participants
@jaraco
Member

jaraco commented Oct 28, 2017

  • What kind of change does this PR introduce? (Bug fix, feature, docs update, ...)

Feature and Bugfix and code removal.

  • What is the related issue number (starting with #)

#1231

  • What is the current behavior? (You can also link to an open issue here)

CherryPy has its own limited implementation of a Case-Insensitive Dict.

  • What is the new behavior (if this is a feature change)?

I was reviewing #1633, I realized I'd seen this pattern before... and built a general-purpose class for its purpose... and released it as a library for others to consume. This approach removes CherryPy's bespoke implementation, which has the reported deficiency, and replaces it with the proven class found in jaraco.collections.

@codacy-bot

This comment has been minimized.

codacy-bot commented Oct 28, 2017

Codacy Here is an overview of what got changed by this pull request:

Issues
======
+ Solved 2
- Added 2
           

See the complete overview on Codacy

@@ -377,49 +379,16 @@ def parse_query_string(query_string, keep_blank_values=True, encoding='utf-8'):
return pm
class CaseInsensitiveDict(dict):
class CaseInsensitiveDict(jaraco.collections.KeyTransformingDict):

This comment has been minimized.

@codacy-bot
def pop(self, key, default):
return dict.pop(self, str(key).title(), default)
@staticmethod
def transform_key(key):

This comment has been minimized.

@codacy-bot

This comment has been minimized.

@webknjaz

webknjaz Oct 29, 2017

Member

I think, we could invent better name: this one doesn't give an idea of what's going on here.

This comment has been minimized.

@jaraco

jaraco Oct 29, 2017

Member

Feel free to file a ticket/PR with jaraco.collections.

@jaraco

In light of #1671, this dependency is not allowed, so the functionality must be inlined before this PR can move forward in the current master.

@codecov

This comment has been minimized.

codecov bot commented Dec 17, 2017

Codecov Report

Merging #1654 into master will increase coverage by 0.86%.
The diff coverage is 81.25%.

@@            Coverage Diff            @@
##           master   #1654      +/-   ##
=========================================
+ Coverage   78.04%   78.9%   +0.86%     
=========================================
  Files         106     106              
  Lines       13798   15761    +1963     
=========================================
+ Hits        10769   12437    +1668     
- Misses       3029    3324     +295

@jaraco jaraco force-pushed the feature/reuse-case-insensitive-dict branch from 08c1e5f to 5da63b7 Dec 17, 2017

@jaraco jaraco force-pushed the feature/reuse-case-insensitive-dict branch from 5da63b7 to 3bcefda Dec 17, 2017

@jaraco jaraco merged commit 0e45422 into master Dec 17, 2017

1 of 7 checks passed

ci/circleci CircleCI is running your tests
Details
continuous-integration/appveyor/branch Waiting for AppVeyor build to complete
Details
continuous-integration/appveyor/pr Waiting for AppVeyor build to complete
Details
continuous-integration/travis-ci/pr The Travis CI build is in progress
Details
continuous-integration/travis-ci/push The Travis CI build is in progress
Details
lgtm analysis: Python Running analyses for revisions
Details
WIP ready for review
Details

@jaraco jaraco deleted the feature/reuse-case-insensitive-dict branch Dec 17, 2017

@webknjaz

This comment has been minimized.

Member

webknjaz commented Dec 17, 2017

This pull request fixes 1 alert - view on lgtm.com

fixed alerts:

  • 1 for Non-callable called

Comment posted by lgtm.com

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment