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

BC: Simplify SMW ask output structure #173

Merged
merged 1 commit into from Aug 2, 2019
Merged

BC: Simplify SMW ask output structure #173

merged 1 commit into from Aug 2, 2019

Conversation

danmichaelo
Copy link
Member

I don't really understand why the ask() method yields key-value pairs (yield {key: value}) rather than just the values (yield value). Since they are yielded from a generator, the keys cannot be used for indexing, so I wonder if this was just a mistake, and I'm tempted to simplify it to yield just the values (result objects), but this will be a breaking change. Do you have any opinions on this, @Ubibene ?

As I was querying a site with SMW 2.3.0 I also learned that the format of the results changed either in SMW 2.4.0 or 2.5.0, so I'm adding a check to support both the old format and the new.

@ghost
Copy link

ghost commented Oct 23, 2017

Hi
I'll have a look this week-end.

I remember though that I needed both key and value data.

I'll keep you posted

@ghost
Copy link

ghost commented Oct 24, 2017

Ok , issue #156 ( #156 ) explains why I needed both keys and values from the generator

@danmichaelo
Copy link
Member Author

Not sure if I get you. The problem you desribed in #156 was that it only returned keys, not values. But if we return only values, the key is still part of the value (as value['fulltext']), so it should be ok? You could then still get the key:

for result in site.ask('test'):
    key = result['fulltext']
    ...

(And looking up by key, something like site.ask('test')[key], never worked anyways, since the result wasn't a dict, but a list of dicts.)

@ghost
Copy link

ghost commented Oct 25, 2017

You may be wright Dan.I'm a python newbie and my code is probably far from perfect. Unfortunately as I have changed jobs, I don't have access to "my" code and can't be sure if it will work with minor modifications if you change mwclient the way you suggest.

@danmichaelo
Copy link
Member Author

Ok, no prob, just wanted to check with you so I didn't mess up your project!

My guess is that the method is not used much since it was added quite recently to mwclient, but I'll leave this PR open for some time in case someone else want to comment.

Breaking change: Change the `Site.ask()` method to produce less nested
output. Basically, change `yield {title: answer}` to `yield answer`. The
title is included in the answer object after all.
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.

None yet

1 participant