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

Generators in charts don't let me iterate more than once #5

Closed
atl opened this issue Mar 30, 2012 · 5 comments
Closed

Generators in charts don't let me iterate more than once #5

atl opened this issue Mar 30, 2012 · 5 comments

Comments

@atl
Copy link
Contributor

atl commented Mar 30, 2012

Generator expressions are cool and all, but I think they're out of place in the Chart.<chart_type> attribute for a couple reasons.

  1. They are one-shot. For example:
> c = mmpy.Chart('bb789492225c4c4da2e15f617acc9982')
> list(c.artist)
[(1, None, <mmpy.artist.Artist at 0x106dd6f50>),
 ...]
> list(c.artist)
[]
  1. They are based on the volatile self.response attribute, which, if it (eventually) follows mmpy.Artist()'s patterns, changes value with each API call.

My personal opinion would be to use a simple list comprehension in the place of the generator expression, so that at least the c.artist attribute has stable behaviour, but I don't know where y'all are headed with this.

@gearmonkey
Copy link
Contributor

The generator versus list issue in this issue is done for two reasons, though there are perhaps ways around both.

  1. memory footprints - this is really not a major issue, given the current size of charts, but if they're very large...
  2. response time - for current chart entities (artist) making a class instance doesn't call the api, but for others this may not be the case, suggesting a benefit to creating the object when used rather than when the chart is created.

Though now that I'm writing these reasons out, it occurs to me that this all may be adding up to a bit too much support for things that don't exist, since given the current calls I agree that a list comprehension makes more sense...

@atl
Copy link
Contributor Author

atl commented Mar 30, 2012

I totally get where you're coming from, especially the ideas behind late binding for the purposes of reducing memory footprint and latency. In addition, I may only want the top 10 in a chart, and any other data is unnecessary (but note that I can't slice the generator anyway).

[So what we really need are Python closures, so that the Artist object instantiation can be late-bound instead of the list?]

Hum, I mostly brought it up to get at what you were thinking, and to advocate for dead-simple repeatability in a user-facing API.

@gearmonkey
Copy link
Contributor

@atl :
[So what we really need are Python closures, so that the Artist object instantiation can be late-bound instead of the list?]

yes. I suppose the question then is which comprehension is a better approximation of what is desired, as a placeholder until such time as someone (probably me) can write something that looks a lot like closures...

@atl
Copy link
Contributor Author

atl commented Mar 30, 2012

I was playing with some "how big is an Object" code in python, but calling vars() on a pre-API call Artist() shows how compact that object really is.

If you take a policy stand that other entities shall be as minimal as that (and hitting the API is a separate step), I'd place my vote for list comprehension.

@gearmonkey
Copy link
Contributor

Alright, you've convinced me, until such time I (or some else) gets some proper closures in the charts class we'll change it over to a list comprehension, and to keep core entities small till you do something with them.

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

No branches or pull requests

2 participants