-
Notifications
You must be signed in to change notification settings - Fork 397
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
Added support for Twitter Ads API #403
base: master
Are you sure you want to change the base?
Conversation
The code looks fine, but as you said, the ad stuff was split off into different files -- I'd rather just have the merge of the files done in this branch before merging upstream. Is there any difference in You can just use the Also, you can move all the endpoint methods from Something like this: class EndpointsMixin(object):
# Timelines
def get_mentions_timeline(self, **params):
...
...
# Ads
def get_accounts(self, **params):
response = self.get('accounts', params=params)
return response['data'] Changing these, you might have to change some of the tests (even though they'll be skipped) Last note, normally, I would question why the tests were written in the old form (against the real api, we moved to using Again, everything looks pretty solid. Just asking to merge, delete those files and rearrange code from the files mentioned :) |
Hi Michael! Sorry for a quite late response, I had some other stuff to do and didn't have time to look into this. Also, I decided to keep the "endpoints_ads.py" and "endpoints.py" files separate in order to keep the functions of Twitter Ads API and Twitter API separate. If you still think I should merge them, please let me know and I'll do that. :) Regarding the tests, I used the integration testing during the implementation of Twitter Ads API, and unfortunately didn't have time to add unit tests as well. Hope this isn't too big of a problem... |
Sorry, I've been super busy. Still wanting to get this merged!
Also, I'm trying to think of a more intuitive way for someone to use the API Instead of t = Twython(api_type=Twython.API_TYPE_TWITTER_ADS) Maybe extending Twython: https://gist.github.com/michaelhelmick/7cd75a4cfacf795c497d to two separate classes is best? You won't have to do that. If you can just merge the endpoints into one file. I'll merge this. I'll be making some enhancements. I think we might have a |
Ok, I combined the Ads endpoints into the endpoints.py file. As for the TwythonCore, Twython and TwythonAds idea, I think it definitely makes sense! :) But if I understood you correctly you will do those things, right? |
am i need do apply 3 leves ads api application first? |
Nice PR. 👍 I also like the idea of TwythonCore and TwythonAds. But we should merge this PR then do another one for that. |
Hi @nouvak, So, I'm starting to tackle this and I was wondering, do you find a use case where a use will need to authenticate with Twitter then use Twitter Ads? Do you think it would make more sense to keep the auth stuff in i.e. t = Twython(...)
# do authentication
final_tokens = t.get_final_tokens() # I think that's what it's called? haha
ads = TwythonAds(key, secret, final_tokens['oauth_token'], final_tokens['oauth_token_secret'])
ads.some_method() |
Hi Mike! Hmm, I'm not sure if I understood your question completely. Were you planning on creating a separate TwythonAds class? |
@nouvak in your previous comment, you said:
In response to my gist. I was planning on creating separate classes. |
Oh I see. I totally forgot about your intention to separate the the Twython and TwythonAd class. With regards to your question about authentication functionality, I would implement it in a separate class that would then be used by both, Twython and TwythonAd class. The design I would implement:
This is just a suggestion: you are free to do the implementation the way it makes the most sense to you. |
As this is old and out of sync... if anyone wants to tackle getting it up and running be my guest. |
Hello!!
I added a subset of the Twitter Ads API (https://dev.twitter.com/ads/overview) to Twython.
The most commonly used functions for managing campaigns and creatives, as well as one function for the retrieval of statistics were added.
I also implemented the integration tests which I used to make sure the API works properly.
If you think patch provides an added value to Twython, I would be happy if you included it to the main branch. I tried to follow the coding style as much as possible, however if there are some issues in the code, please let me know.
Note: There is some duplication of the code in api.py and api_ads.py: for the first part, I didn't want to mess with the "api.py" stuff, so I simply created a copy of the file and adapted it to the Twitter Ads API. With minimal rewrite, those files could be merged together. This could be done later.