Comments (16)
Sorry for the typos, I didn't really proofread what I was writing :)
I also modified the GetDirectMessages in the same way as above.
Original comment by sam%[email protected]
on 27 Sep 2008 at 7:55
from python-twitter.
Thanks for the patch! I definitely support the goal of supporting all
parameters.
But is a dictionary keyed by strings the best approach? What if we had
enumerated
constants for each key? Something like:
parameters = { Parameters.SINCE_ID: 932442808,
Parameters.COUNT: 10,
# ... }
That said, I think I'd still prefer that we just update the method signatures
directly. That's far more discoverable, don't you think?
-DeWitt
Original comment by dclinton
on 27 Sep 2008 at 8:06
from python-twitter.
While leaving it as-is might be easier for the developer and make sense now,
the code
will require regular maintenance over time. I'm not sure if it'll happen
(that's not
intended to be an insult to you - I know this is an open source project and
you're
only one person). For example, issue 22 has been around for months, and I'm
pretty
sure twitter changed their api months before that.
While I probably wouldn't want to do this, passing the parameter dictionary
isn't
necessarily mutually exclusive with the existing implementation. Maintaining
both
would be somewhat inelegant, but it'd at least allow users to make use of new
parameters as soon as twitter releases them. Users looking for basic API usage
could
still continue to use python-twitter in its current form.
Also - I'm not an expert in python but it seems like passing arguments within a
dictionary seems relatively common. For example, django does it in their url
dispatcher:
http://docs.djangoproject.com/en/dev/topics/http/urls/?from=olddocs#passing-extr
a-options-to-view-functions
Original comment by sam%[email protected]
on 27 Sep 2008 at 10:44
from python-twitter.
Not a bad point -- they aren't mutually exclusive. We could do something like:
GetUserTimeline(user = 'foo',
since_id = 932442808,
extra_parameters = { 'count': 10 })
And everything passed in as 'extra_parameters' would automatically be included
in the
request.
Sounds good?
Original comment by dclinton
on 29 Sep 2008 at 4:55
from python-twitter.
that sounds great. the above patch would work as described with minor changes.
Would you also like to also pass since_id directly whenever possible (ie, issue
22)?
I'll gladly update the patch... although you may need to rewrite the
documentation
(since I'm not sure how that works)
Original comment by [email protected]
on 29 Sep 2008 at 3:10
from python-twitter.
Cool. I feel like we should still try and put every parameter we know about
(like
since_id) into the signature of each API call, and rely on extra_parameters
only for
those that we haven't yet exposed explicitly.
Because we could go either way on this, lets say that the extra_parameters dict
overrides the explicit parameters. I.e., extra_parameters is overlaid on top
of the
parameters dict that was initialized via the explicit parameters.
Original comment by dclinton
on 29 Sep 2008 at 3:37
from python-twitter.
Ok sounds good. I'll update the patch.
If it's more elegant to make the parameters dict override extra_parameters -
would
you prefer that path?
Original comment by [email protected]
on 30 Sep 2008 at 6:18
from python-twitter.
I'm not sure which is more intuitive for users. What do you think? We should
document it clearly no matter what.
Code wise it should be the same either way, either initializing the parameter
dict
with extra_parameters first, or updating it at the end.
Original comment by dclinton
on 30 Sep 2008 at 6:24
from python-twitter.
Personally, I feel like if you're explicitly passing the method a parameter it
should
override what's in the dictionary.
Also, this would allow for the following code:
def GetPublicTimeline(self, extra_parameters={}, since_id=None):
if since_id:
extra_parameters['since_id'] = since_id
url = 'http://twitter.com/statuses/public_timeline.json'
json = self._FetchUrl(url, parameters=extra_parameters)
data = simplejson.loads(json)
return [Status.NewFromJsonDict(x) for x in data]
Original comment by [email protected]
on 30 Sep 2008 at 6:33
from python-twitter.
I'm okay with the decision to use extra_parameters as the initial values.
However, for the sake of protecting the mutable parameter, please consider
writing
that as:
def GetPublicTimeline(self, since_id=None, extra_parameters=None):
if extra_parameters:
parameters = dict(extract_parameters)
else:
parameters = {}
if since_id:
parameters['since_id'] = since_id
url = 'http://twitter.com/statuses/public_timeline.json'
json = self._FetchUrl(url, parameters=parameters)
# ...
Original comment by dclinton
on 30 Sep 2008 at 6:54
from python-twitter.
Ok.. what about:
def GetPublicTimeline(self, since_id=None, extra_parameters={}):
parameters = extra_parameters
if since_id:
parameters['since_id'] = since_id
...
Original comment by [email protected]
on 30 Sep 2008 at 7:03
from python-twitter.
No, that would still leave the argument open to modification, which is what the
defensive copy prevents.
In fact, there are two bugs there.
Here's the first bug:
>>> my_params = {'since_id': 123}
>>> my_params['since_id']
123
>>> api.GetPublicTimeline(since_id = 456, extra_parameters=my_params)
>>> my_params['since_id']
456
Note that calling GetPublicTimeline modified the dict that was passed in. Not
good.
Generally speaking you should always make defensive copies of mutable
parameters, but
especially in the case where you know you're going to be modifying them.
But the other bug is even worse. The "extra_parameters={}" in the parameter
list is
the problem, as the exact same default dict will be reused between calls, and
it will
be modified each time.
For example:
>>> def danger(d={}, a=None):
... if a:
... d['a'] = a # bug! if d wasn't passed in explicitly, the default {} is
reused and modified over and over
... print d
...
>>> danger()
{}
>>> danger(a=2)
{'a': 2}
>>> danger()
{'a': 2} # whoa! how'd that happen?
Scary, right?
The way to prevent that is to say:
>>> def safe(d=None, a=None):
... if d is None:
... d = {} # safe, because it initializes a new empty dict each time
... if a:
... d['a'] = a
... print d
...
>>> safe()
{}
>>> safe(a=2)
{'a': 2}
>>> safe()
{}
Make sense?
-DeWitt
Original comment by dclinton
on 30 Sep 2008 at 7:30
from python-twitter.
OK - makes sense. Sorry about that, I'm still learning python :)
Original comment by [email protected]
on 30 Sep 2008 at 8:11
from python-twitter.
No problem at all! Happy to help teach it.
Original comment by dclinton
on 30 Sep 2008 at 8:17
from python-twitter.
Probably a month too late, but have you considered using keyword arguments for
the
extra parameters? They pack up the arguments into a new dictionary
automatically.
def GetPublicTimeline(self, **extra_parameters):
url = 'http://twitter.com/statuses/public_timeline.json'
json = self._FetchUrl(url, extra_parameters)
data = simplejson.loads(json)
return [Status.NewFromJsonDict(x) for x in data]
And you can pass them in like you do ordinary parameters:
api.GetPublicTimeline(since_id=123, count=42, etc=1)
No need to make copies of dictionaries, or create temporary ones, or worry about
which 'since_id' parameter takes precedence.
Original comment by [email protected]
on 31 Oct 2008 at 7:43
from python-twitter.
Original comment by dclinton
on 26 Apr 2009 at 5:56
- Changed state: Accepted
from python-twitter.
Related Issues (20)
- Character Limit does not accurately match URL t.co wrapper
- NewFromJsonDict AttributeError HOT 2
- "UnicodeEncodeError: 'ascii' codec can't encode characters" during GetSearch() for utf-8 encoded search terms
- Current head (dca91beb7418) has a failing unit test HOT 1
- 404 - Dependency issue? HOT 1
- README.md did not get up to speed HOT 1
- Pip Install from source fails (google code) HOT 3
- api.GetMentions errors HOT 1
- import twitter produces AttributeError: 'module' object has no attribute 'Http' HOT 1
- Getting {“errors”:[{“message”:“Bad Authentication data”,“code”:215}]} using python-twitter API v1.0 HOT 5
- Direct Messages failing HOT 3
- GetPublicTimeline() is referenced in docs but not implemented HOT 1
- python 3.x HOT 2
- GetUser gives a "page does not exist 34" HOT 2
- remove dependency to python-oauth2 switch to oauthlib HOT 2
- GetUserTimeLine does not work
- status of python-twitter-1.1
- api.CreateList returns UnboundLocalError
- Cannot be installed with Pip HOT 2
- User behind a proxy will face a problem
Recommend Projects
-
React
A declarative, efficient, and flexible JavaScript library for building user interfaces.
-
Vue.js
🖖 Vue.js is a progressive, incrementally-adoptable JavaScript framework for building UI on the web.
-
Typescript
TypeScript is a superset of JavaScript that compiles to clean JavaScript output.
-
TensorFlow
An Open Source Machine Learning Framework for Everyone
-
Django
The Web framework for perfectionists with deadlines.
-
Laravel
A PHP framework for web artisans
-
D3
Bring data to life with SVG, Canvas and HTML. 📊📈🎉
-
Recommend Topics
-
javascript
JavaScript (JS) is a lightweight interpreted programming language with first-class functions.
-
web
Some thing interesting about web. New door for the world.
-
server
A server is a program made to process requests and deliver data to clients.
-
Machine learning
Machine learning is a way of modeling and interpreting data that allows a piece of software to respond intelligently.
-
Visualization
Some thing interesting about visualization, use data art
-
Game
Some thing interesting about game, make everyone happy.
Recommend Org
-
Facebook
We are working to build community through open source technology. NB: members must have two-factor auth.
-
Microsoft
Open source projects and samples from Microsoft.
-
Google
Google ❤️ Open Source for everyone.
-
Alibaba
Alibaba Open Source for everyone
-
D3
Data-Driven Documents codes.
-
Tencent
China tencent open source team.
from python-twitter.