Skip to content
This repository was archived by the owner on Oct 29, 2024. It is now read-only.

Refactored ResultSet #194

Merged
merged 1 commit into from
Jun 15, 2015
Merged

Refactored ResultSet #194

merged 1 commit into from
Jun 15, 2015

Conversation

aviau
Copy link
Collaborator

@aviau aviau commented Jun 14, 2015

Improvements on the ResultSet API to make it more intuitive.

@aviau aviau force-pushed the refactor_resultset branch 3 times, most recently from d641d9d to 64f26ef Compare June 14, 2015 19:29
@aviau aviau changed the title Refactored ResultSet [WIP] Refactored ResultSet Jun 14, 2015
@aviau
Copy link
Collaborator Author

aviau commented Jun 14, 2015

@ChristopherRabotin, @adventures91, @kipe, @weiyoung-zhou, @georgijd

Any feedback?

Should I always return a list of ResultSets? What I am doing right now is that I am returning a list only if there are more than 1 results.

Let me know if you don't wish to be included in those discussions.

@aviau aviau force-pushed the refactor_resultset branch from 64f26ef to cf1ef9b Compare June 14, 2015 19:47
@kipe
Copy link
Contributor

kipe commented Jun 14, 2015

I do like the changes in general, makes the code more pythonic.
Especially the filtering by tags seems more logical by using this kind of structure.

It would be more logical to me, if one query would return a single ResultSet and multiple queries a list of ResultSets.

On the other hand, returning a list always would be constant, but does raise the question on how often is this feature used. And even if it is, the programmer should know he's running multiple queries and should expect multiple results -> knows to expect a list of ResultSets instead of a single ResultSet.

@aviau
Copy link
Collaborator Author

aviau commented Jun 14, 2015

It would be more logical to me, if one query would return a single ResultSet and multiple queries a list of ResultSets.

That's exactly what it does in my PR. If you make multiple queries (separated by ;) in a single client.query() call, you will get a list of ResultSets

but does raise the question on how often is this feature used.

Indeed. Most users will have to get the first element of the list all the time. It would be quite annoying. I would also like to avoid breaking the API so this is why chose not to return a list.

@geodimm
Copy link
Contributor

geodimm commented Jun 15, 2015

I second @kipe's opinion on the filtering :) That way the ResultSet object becomes more flexible and we can easily add more filter parameters in the future. As for the multiple queries result type, what makes most sense to me is that a single query should return a single ResultSet object and multiple queries should return a list of ResultSet objects. From developer's perspective, it's really frustrating to always use expressions like rs[0] to get the data you need when most of the time you execute a single query. It pollutes the code and it's prone to errors.

@aviau
Copy link
Collaborator Author

aviau commented Jun 15, 2015

Thank you for the feedback.

aviau added a commit that referenced this pull request Jun 15, 2015
@aviau aviau merged commit 4be0d6a into master Jun 15, 2015
@aviau aviau deleted the refactor_resultset branch June 15, 2015 12:24
@aviau aviau restored the refactor_resultset branch July 31, 2015 15:21
@aviau aviau deleted the refactor_resultset branch July 31, 2015 15:21
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Multiple query request to same series creates a bad ResultSet
3 participants