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

Make the result class a MutableMapping #7

Closed
wants to merge 2 commits into from

Conversation

Rafiot
Copy link

@Rafiot Rafiot commented Nov 28, 2016

This way, you can also use .get('key') on the parsed result.

@coveralls
Copy link

Coverage Status

Coverage decreased (-0.008%) to 99.659% when pulling 098449a on Rafiot:MutableMapping into 36e4761 on DomainTools:master.

1 similar comment
@coveralls
Copy link

Coverage Status

Coverage decreased (-0.008%) to 99.659% when pulling 098449a on Rafiot:MutableMapping into 36e4761 on DomainTools:master.

@Rafiot
Copy link
Author

Rafiot commented Nov 28, 2016

Fails on Python 2.x... trying out a bunch of stuff.

@Rafiot
Copy link
Author

Rafiot commented Nov 28, 2016

Okay, I'm not sure I understand what _items is supposed to do.

The main reason I wanted to make Results a collections.MutableMapping is because it makes the class a real dict and also gives me a get method. I could implement this one manually, but having Results a dictionary would be very nice and more generic.
Can you have a look, please?

@timothycrosley
Copy link
Contributor

Hi @Rafiot,

I like what you are trying to accomplish here! I think some of the confusion comes from the fact that the base result object is intended to be interacted with not only as a dictionary but as a list in several cases. Additionally, in 3.5+ the location of the base collection objects has changed. I've put together a pull request here: https://github.com/DomainTools/python_api/pull/8/files#diff-0a50901a753def4eb9add20ff0f9817bL1 that I believe accomplishes what you are after.

Thanks!

~Timothy

@Rafiot
Copy link
Author

Rafiot commented Nov 28, 2016

Hi @timothycrosley,

Yep, it does exactly what I wanted to do, thanks!

Raphaël

@Rafiot Rafiot closed this Nov 28, 2016
@timothycrosley
Copy link
Contributor

Awesome! I've going ahead and pushed out version 0.1.5 to PYPI with this improvement in place.

Thanks!

~Timothy

@Rafiot
Copy link
Author

Rafiot commented Nov 28, 2016

Excellent, I'm going to use it in the misp-modules: MISP/misp-modules#75

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.

3 participants