Skip to content

Generalize subscripts using collections.abc module#23

Merged
dmlb2000 merged 1 commit intopacifica:masterfrom
markborkum:feat/collections-abc-subscripts
May 20, 2019
Merged

Generalize subscripts using collections.abc module#23
dmlb2000 merged 1 commit intopacifica:masterfrom
markborkum:feat/collections-abc-subscripts

Conversation

@markborkum
Copy link
Copy Markdown
Contributor

Description

Replaces isinstance(x, dict) with isinstance(x, collections.abc.Mapping).

Replaces isinstance(x, list) with isinstance(x, collections.abc.Sequence) and not isinstance(x, str).

Issues Resolved

Closes #20

Check List

  • All tests pass.
  • New functionality includes testing.
  • New functionality has been documented in the README if applicable

@markborkum markborkum requested a review from dmlb2000 May 20, 2019 19:42
Copy link
Copy Markdown
Member

@dmlb2000 dmlb2000 left a comment

Choose a reason for hiding this comment

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

This looks fine by me, I like how you didn't have the change the test suite to fix this. Might consider adding another pull request to enhance the test suite with example inherited classes to show the functionality @niflostancu is talking about in his issue.

@dmlb2000
Copy link
Copy Markdown
Member

@niflostancu Does this feature meet your needs?

@niflostancu
Copy link
Copy Markdown

Wow, that was fast. It's perfect for my use case, and, possibly, others.
Thanks!

@dmlb2000 dmlb2000 merged commit 2d74af2 into pacifica:master May 20, 2019
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.

Support for collections.abc classes

3 participants