|
|
Created:
11 years, 11 months ago by dhermes Modified:
11 years, 9 months ago CC:
google-api-python-client_googlegroups.com Visibility:
Public. |
Patch Set 1 #
Total comments: 2
Patch Set 2 : Removing commented out code that was left in by mistake. #
Total comments: 8
Patch Set 3 : Addressing review comments from jcgregorio. #Patch Set 4 : Changing attr cache to just keep track of the dynamic attributes. #Patch Set 5 : Adding docstring to _set_dynamic_attr method. #Patch Set 6 : Changed indent on docstring in methodNext, was causing parser to fail in describe.method_params. #Patch Set 7 : Fixing pickle tests in discovery after changing from _attr_cache to _dynamic_attr list. #Patch Set 8 : Changing setup.py to target a newer version of httplib2. #Patch Set 9 : Rebasing after several commits. #
MessagesTotal messages: 24
Sorry this just got sent. I am still not 100% sure what causes rietveld to send these emails. Afshar reviewed this for me the week before last, so he may be worth chatting with as well. On Mon, Oct 15, 2012 at 8:57 PM, <dhermes@google.com> wrote: > Reviewers: jcgregorio_google, Ali Afshar, > > > > Please review this at https://codereview.appspot.**com/6610049/<https://codereview.appspot.com/6610... > > Affected files: > M apiclient/discovery.py > M apiclient/errors.py > M tests/test_discovery.py > > > -- Danny Hermes Developer Programs Engineer
Sign in to reply to this message.
Hey Joe, Now that you're back, will you have a chance to review this? On Mon, Oct 15, 2012 at 8:59 PM, Danny Hermes <dhermes@google.com> wrote: > Sorry this just got sent. I am still not 100% sure what causes rietveld to > send these emails. Afshar reviewed this for me the week before last, so he > may be worth chatting with as well. > > > On Mon, Oct 15, 2012 at 8:57 PM, <dhermes@google.com> wrote: > >> Reviewers: jcgregorio_google, Ali Afshar, >> >> >> >> Please review this at https://codereview.appspot.**com/6610049/<https://codereview.appspot.com/6610... >> >> Affected files: >> M apiclient/discovery.py >> M apiclient/errors.py >> M tests/test_discovery.py >> >> >> > > > -- > Danny Hermes > Developer Programs Engineer > -- Danny Hermes Developer Programs Engineer
Sign in to reply to this message.
https://codereview.appspot.com/6610049/diff/1/tests/test_discovery.py File tests/test_discovery.py (right): https://codereview.appspot.com/6610049/diff/1/tests/test_discovery.py#newcode805 tests/test_discovery.py:805: # http = HttpMock(datafile('plus.json'), {'status': '200'}) Leftover commented out code?
Sign in to reply to this message.
https://codereview.appspot.com/6610049/diff/1/tests/test_discovery.py File tests/test_discovery.py (right): https://codereview.appspot.com/6610049/diff/1/tests/test_discovery.py#newcode805 tests/test_discovery.py:805: # http = HttpMock(datafile('plus.json'), {'status': '200'}) On 2012/10/25 18:41:59, jcgregorio_google wrote: > Leftover commented out code? Yup. Will get rid of it.
Sign in to reply to this message.
Any progress on this review? On Thu, Oct 25, 2012 at 11:43 AM, <dhermes@google.com> wrote: > > https://codereview.appspot.**com/6610049/diff/1/tests/test_**discovery.py<htt... > File tests/test_discovery.py (right): > > https://codereview.appspot.**com/6610049/diff/1/tests/test_** > discovery.py#newcode805<https://codereview.appspot.com/6610049/diff/1/tests/test_discovery.py#newcode805> > tests/test_discovery.py:805: # http = HttpMock(datafile('plus.json')**, > {'status': '200'}) > On 2012/10/25 18:41:59, jcgregorio_google wrote: > >> Leftover commented out code? >> > > Yup. Will get rid of it. > > https://codereview.appspot.**com/6610049/<https://codereview.appspot.com/6610... > -- Danny Hermes Developer Programs Engineer
Sign in to reply to this message.
https://codereview.appspot.com/6610049/diff/8001/apiclient/discovery.py File apiclient/discovery.py (right): https://codereview.appspot.com/6610049/diff/8001/apiclient/discovery.py#newco... apiclient/discovery.py:384: one blank line https://codereview.appspot.com/6610049/diff/8001/apiclient/discovery.py#newco... apiclient/discovery.py:634: previous_response: The response from the request for the previous page. (required) 80 chars https://codereview.appspot.com/6610049/diff/8001/apiclient/discovery.py#newco... apiclient/discovery.py:670: def __init__(self, http=None, baseUrl=None, model=None, developerKey=None, I don't think any of these needs =None defaults. https://codereview.appspot.com/6610049/diff/8001/apiclient/discovery.py#newco... apiclient/discovery.py:739: def set_service_methods(self): _set_service_methods
Sign in to reply to this message.
I have already updated the new patch set. https://codereview.appspot.com/6610049/diff/8001/apiclient/discovery.py File apiclient/discovery.py (right): https://codereview.appspot.com/6610049/diff/8001/apiclient/discovery.py#newco... apiclient/discovery.py:384: On 2012/10/30 18:03:04, jcgregorio_google wrote: > one blank line Done. https://codereview.appspot.com/6610049/diff/8001/apiclient/discovery.py#newco... apiclient/discovery.py:634: previous_response: The response from the request for the previous page. (required) On 2012/10/30 18:03:04, jcgregorio_google wrote: > 80 chars Done. https://codereview.appspot.com/6610049/diff/8001/apiclient/discovery.py#newco... apiclient/discovery.py:670: def __init__(self, http=None, baseUrl=None, model=None, developerKey=None, On 2012/10/30 18:03:04, jcgregorio_google wrote: > I don't think any of these needs =None defaults. You are right. I'm not sure why I changed this, and also accidentally changed the order of developerKey and requestBuilder. https://codereview.appspot.com/6610049/diff/8001/apiclient/discovery.py#newco... apiclient/discovery.py:739: def set_service_methods(self): On 2012/10/30 18:03:04, jcgregorio_google wrote: > _set_service_methods Done.
Sign in to reply to this message.
Have you also verified that this doesn't change the output of 'make docs' ? -joe On Tue, Oct 30, 2012 at 2:12 PM, <dhermes@google.com> wrote: > I have already updated the new patch set. > > > > https://codereview.appspot.com/6610049/diff/8001/apiclient/discovery.py > File apiclient/discovery.py (right): > > https://codereview.appspot.com/6610049/diff/8001/apiclient/discovery.py#newco... > apiclient/discovery.py:384: > On 2012/10/30 18:03:04, jcgregorio_google wrote: >> >> one blank line > > > Done. > > > https://codereview.appspot.com/6610049/diff/8001/apiclient/discovery.py#newco... > apiclient/discovery.py:634: previous_response: The response from the > request for the previous page. (required) > On 2012/10/30 18:03:04, jcgregorio_google wrote: >> >> 80 chars > > > Done. > > > https://codereview.appspot.com/6610049/diff/8001/apiclient/discovery.py#newco... > apiclient/discovery.py:670: def __init__(self, http=None, baseUrl=None, > model=None, developerKey=None, > On 2012/10/30 18:03:04, jcgregorio_google wrote: >> >> I don't think any of these needs =None defaults. > > > You are right. I'm not sure why I changed this, and also accidentally > changed the order of developerKey and requestBuilder. > > > https://codereview.appspot.com/6610049/diff/8001/apiclient/discovery.py#newco... > apiclient/discovery.py:739: def set_service_methods(self): > On 2012/10/30 18:03:04, jcgregorio_google wrote: >> >> _set_service_methods > > > Done. > > https://codereview.appspot.com/6610049/
Sign in to reply to this message.
Nope. How might one do this? On Tue, Oct 30, 2012 at 11:17 AM, Joe Gregorio <jcgregorio@google.com>wrote: > Have you also verified that this doesn't change the output of 'make docs' ? > > -joe > > On Tue, Oct 30, 2012 at 2:12 PM, <dhermes@google.com> wrote: > > I have already updated the new patch set. > > > > > > > > https://codereview.appspot.com/6610049/diff/8001/apiclient/discovery.py > > File apiclient/discovery.py (right): > > > > > https://codereview.appspot.com/6610049/diff/8001/apiclient/discovery.py#newco... > > apiclient/discovery.py:384: > > On 2012/10/30 18:03:04, jcgregorio_google wrote: > >> > >> one blank line > > > > > > Done. > > > > > > > https://codereview.appspot.com/6610049/diff/8001/apiclient/discovery.py#newco... > > apiclient/discovery.py:634: previous_response: The response from the > > request for the previous page. (required) > > On 2012/10/30 18:03:04, jcgregorio_google wrote: > >> > >> 80 chars > > > > > > Done. > > > > > > > https://codereview.appspot.com/6610049/diff/8001/apiclient/discovery.py#newco... > > apiclient/discovery.py:670: def __init__(self, http=None, baseUrl=None, > > model=None, developerKey=None, > > On 2012/10/30 18:03:04, jcgregorio_google wrote: > >> > >> I don't think any of these needs =None defaults. > > > > > > You are right. I'm not sure why I changed this, and also accidentally > > changed the order of developerKey and requestBuilder. > > > > > > > https://codereview.appspot.com/6610049/diff/8001/apiclient/discovery.py#newco... > > apiclient/discovery.py:739: def set_service_methods(self): > > On 2012/10/30 18:03:04, jcgregorio_google wrote: > >> > >> _set_service_methods > > > > > > Done. > > > > https://codereview.appspot.com/6610049/ > -- Danny Hermes Developer Programs Engineer
Sign in to reply to this message.
There's a Makefile in the project. Just run: $ make docs Then look at the diffs in the docs that produces. You probably want to patch this CL into another copy of the repository since it will modify the doc files which you probably don't want as a part of this CL. Thanks, -joe On Tue, Oct 30, 2012 at 2:34 PM, Danny Hermes <dhermes@google.com> wrote: > Nope. How might one do this? > > > On Tue, Oct 30, 2012 at 11:17 AM, Joe Gregorio <jcgregorio@google.com> > wrote: >> >> Have you also verified that this doesn't change the output of 'make docs' >> ? >> >> -joe >> >> On Tue, Oct 30, 2012 at 2:12 PM, <dhermes@google.com> wrote: >> > I have already updated the new patch set. >> > >> > >> > >> > https://codereview.appspot.com/6610049/diff/8001/apiclient/discovery.py >> > File apiclient/discovery.py (right): >> > >> > >> > https://codereview.appspot.com/6610049/diff/8001/apiclient/discovery.py#newco... >> > apiclient/discovery.py:384: >> > On 2012/10/30 18:03:04, jcgregorio_google wrote: >> >> >> >> one blank line >> > >> > >> > Done. >> > >> > >> > >> > https://codereview.appspot.com/6610049/diff/8001/apiclient/discovery.py#newco... >> > apiclient/discovery.py:634: previous_response: The response from the >> > request for the previous page. (required) >> > On 2012/10/30 18:03:04, jcgregorio_google wrote: >> >> >> >> 80 chars >> > >> > >> > Done. >> > >> > >> > >> > https://codereview.appspot.com/6610049/diff/8001/apiclient/discovery.py#newco... >> > apiclient/discovery.py:670: def __init__(self, http=None, baseUrl=None, >> > model=None, developerKey=None, >> > On 2012/10/30 18:03:04, jcgregorio_google wrote: >> >> >> >> I don't think any of these needs =None defaults. >> > >> > >> > You are right. I'm not sure why I changed this, and also accidentally >> > changed the order of developerKey and requestBuilder. >> > >> > >> > >> > https://codereview.appspot.com/6610049/diff/8001/apiclient/discovery.py#newco... >> > apiclient/discovery.py:739: def set_service_methods(self): >> > On 2012/10/30 18:03:04, jcgregorio_google wrote: >> >> >> >> _set_service_methods >> > >> > >> > Done. >> > >> > https://codereview.appspot.com/6610049/ > > > > > -- > Danny Hermes > Developer Programs Engineer
Sign in to reply to this message.
It does change things. For example it is removing all the subresources from specific API docs. This is likely because the attr is not set on the object so object.__dict__ does not get updated and things like dir(service_object) won't list the subresources. I can set this in a method that will also set it in _attr_cache or just track the attributes that would have ended up in attr_cache via some other mechanism. What do you think? On Tue, Oct 30, 2012 at 11:37 AM, Joe Gregorio <jcgregorio@google.com>wrote: > There's a Makefile in the project. Just run: > > $ make docs > > Then look at the diffs in the docs that produces. You probably want to > patch this CL into > another copy of the repository since it will modify the doc files > which you probably don't want > as a part of this CL. > > Thanks, > -joe > > On Tue, Oct 30, 2012 at 2:34 PM, Danny Hermes <dhermes@google.com> wrote: > > Nope. How might one do this? > > > > > > On Tue, Oct 30, 2012 at 11:17 AM, Joe Gregorio <jcgregorio@google.com> > > wrote: > >> > >> Have you also verified that this doesn't change the output of 'make > docs' > >> ? > >> > >> -joe > >> > >> On Tue, Oct 30, 2012 at 2:12 PM, <dhermes@google.com> wrote: > >> > I have already updated the new patch set. > >> > > >> > > >> > > >> > > https://codereview.appspot.com/6610049/diff/8001/apiclient/discovery.py > >> > File apiclient/discovery.py (right): > >> > > >> > > >> > > https://codereview.appspot.com/6610049/diff/8001/apiclient/discovery.py#newco... > >> > apiclient/discovery.py:384: > >> > On 2012/10/30 18:03:04, jcgregorio_google wrote: > >> >> > >> >> one blank line > >> > > >> > > >> > Done. > >> > > >> > > >> > > >> > > https://codereview.appspot.com/6610049/diff/8001/apiclient/discovery.py#newco... > >> > apiclient/discovery.py:634: previous_response: The response from the > >> > request for the previous page. (required) > >> > On 2012/10/30 18:03:04, jcgregorio_google wrote: > >> >> > >> >> 80 chars > >> > > >> > > >> > Done. > >> > > >> > > >> > > >> > > https://codereview.appspot.com/6610049/diff/8001/apiclient/discovery.py#newco... > >> > apiclient/discovery.py:670: def __init__(self, http=None, > baseUrl=None, > >> > model=None, developerKey=None, > >> > On 2012/10/30 18:03:04, jcgregorio_google wrote: > >> >> > >> >> I don't think any of these needs =None defaults. > >> > > >> > > >> > You are right. I'm not sure why I changed this, and also accidentally > >> > changed the order of developerKey and requestBuilder. > >> > > >> > > >> > > >> > > https://codereview.appspot.com/6610049/diff/8001/apiclient/discovery.py#newco... > >> > apiclient/discovery.py:739: def set_service_methods(self): > >> > On 2012/10/30 18:03:04, jcgregorio_google wrote: > >> >> > >> >> _set_service_methods > >> > > >> > > >> > Done. > >> > > >> > https://codereview.appspot.com/6610049/ > > > > > > > > > > -- > > Danny Hermes > > Developer Programs Engineer > -- Danny Hermes Developer Programs Engineer
Sign in to reply to this message.
On Tue, Oct 30, 2012 at 3:33 PM, Danny Hermes <dhermes@google.com> wrote: > It does change things. For example it is removing all the subresources from > specific API docs. > > This is likely because the attr is not set on the object so object.__dict__ > does not get updated and things like dir(service_object) won't list the > subresources. Yeah, definitely want to keep the dir(service_object) behavior unchanged. > I can set this in a method that will also set it in _attr_cache or just > track the attributes that would have ended up in attr_cache via some other > mechanism. Or would it be easier to invert the meaning of attr_cache and have it just be the name of attributes that will be stripped out of the response from __getstate__? That way you can also avoid over-riding __getattribute__. -joe > > What do you think? > > > On Tue, Oct 30, 2012 at 11:37 AM, Joe Gregorio <jcgregorio@google.com> > wrote: >> >> There's a Makefile in the project. Just run: >> >> $ make docs >> >> Then look at the diffs in the docs that produces. You probably want to >> patch this CL into >> another copy of the repository since it will modify the doc files >> which you probably don't want >> as a part of this CL. >> >> Thanks, >> -joe >> >> On Tue, Oct 30, 2012 at 2:34 PM, Danny Hermes <dhermes@google.com> wrote: >> > Nope. How might one do this? >> > >> > >> > On Tue, Oct 30, 2012 at 11:17 AM, Joe Gregorio <jcgregorio@google.com> >> > wrote: >> >> >> >> Have you also verified that this doesn't change the output of 'make >> >> docs' >> >> ? >> >> >> >> -joe >> >> >> >> On Tue, Oct 30, 2012 at 2:12 PM, <dhermes@google.com> wrote: >> >> > I have already updated the new patch set. >> >> > >> >> > >> >> > >> >> > >> >> > https://codereview.appspot.com/6610049/diff/8001/apiclient/discovery.py >> >> > File apiclient/discovery.py (right): >> >> > >> >> > >> >> > >> >> > https://codereview.appspot.com/6610049/diff/8001/apiclient/discovery.py#newco... >> >> > apiclient/discovery.py:384: >> >> > On 2012/10/30 18:03:04, jcgregorio_google wrote: >> >> >> >> >> >> one blank line >> >> > >> >> > >> >> > Done. >> >> > >> >> > >> >> > >> >> > >> >> > https://codereview.appspot.com/6610049/diff/8001/apiclient/discovery.py#newco... >> >> > apiclient/discovery.py:634: previous_response: The response from the >> >> > request for the previous page. (required) >> >> > On 2012/10/30 18:03:04, jcgregorio_google wrote: >> >> >> >> >> >> 80 chars >> >> > >> >> > >> >> > Done. >> >> > >> >> > >> >> > >> >> > >> >> > https://codereview.appspot.com/6610049/diff/8001/apiclient/discovery.py#newco... >> >> > apiclient/discovery.py:670: def __init__(self, http=None, >> >> > baseUrl=None, >> >> > model=None, developerKey=None, >> >> > On 2012/10/30 18:03:04, jcgregorio_google wrote: >> >> >> >> >> >> I don't think any of these needs =None defaults. >> >> > >> >> > >> >> > You are right. I'm not sure why I changed this, and also accidentally >> >> > changed the order of developerKey and requestBuilder. >> >> > >> >> > >> >> > >> >> > >> >> > https://codereview.appspot.com/6610049/diff/8001/apiclient/discovery.py#newco... >> >> > apiclient/discovery.py:739: def set_service_methods(self): >> >> > On 2012/10/30 18:03:04, jcgregorio_google wrote: >> >> >> >> >> >> _set_service_methods >> >> > >> >> > >> >> > Done. >> >> > >> >> > https://codereview.appspot.com/6610049/ >> > >> > >> > >> > >> > -- >> > Danny Hermes >> > Developer Programs Engineer > > > > > -- > Danny Hermes > Developer Programs Engineer
Sign in to reply to this message.
Yes. That is what I meant. Will update with a patch. On Tue, Oct 30, 2012 at 12:43 PM, Joe Gregorio <jcgregorio@google.com>wrote: > On Tue, Oct 30, 2012 at 3:33 PM, Danny Hermes <dhermes@google.com> wrote: > > It does change things. For example it is removing all the subresources > from > > specific API docs. > > > > This is likely because the attr is not set on the object so > object.__dict__ > > does not get updated and things like dir(service_object) won't list the > > subresources. > > Yeah, definitely want to keep the dir(service_object) behavior unchanged. > > > I can set this in a method that will also set it in _attr_cache or just > > track the attributes that would have ended up in attr_cache via some > other > > mechanism. > > Or would it be easier to invert the meaning of attr_cache and have it > just be the name of attributes that will be stripped out of the response > from > __getstate__? That way you can also avoid over-riding __getattribute__. > > -joe > > > > > What do you think? > > > > > > On Tue, Oct 30, 2012 at 11:37 AM, Joe Gregorio <jcgregorio@google.com> > > wrote: > >> > >> There's a Makefile in the project. Just run: > >> > >> $ make docs > >> > >> Then look at the diffs in the docs that produces. You probably want to > >> patch this CL into > >> another copy of the repository since it will modify the doc files > >> which you probably don't want > >> as a part of this CL. > >> > >> Thanks, > >> -joe > >> > >> On Tue, Oct 30, 2012 at 2:34 PM, Danny Hermes <dhermes@google.com> > wrote: > >> > Nope. How might one do this? > >> > > >> > > >> > On Tue, Oct 30, 2012 at 11:17 AM, Joe Gregorio <jcgregorio@google.com > > > >> > wrote: > >> >> > >> >> Have you also verified that this doesn't change the output of 'make > >> >> docs' > >> >> ? > >> >> > >> >> -joe > >> >> > >> >> On Tue, Oct 30, 2012 at 2:12 PM, <dhermes@google.com> wrote: > >> >> > I have already updated the new patch set. > >> >> > > >> >> > > >> >> > > >> >> > > >> >> > > https://codereview.appspot.com/6610049/diff/8001/apiclient/discovery.py > >> >> > File apiclient/discovery.py (right): > >> >> > > >> >> > > >> >> > > >> >> > > https://codereview.appspot.com/6610049/diff/8001/apiclient/discovery.py#newco... > >> >> > apiclient/discovery.py:384: > >> >> > On 2012/10/30 18:03:04, jcgregorio_google wrote: > >> >> >> > >> >> >> one blank line > >> >> > > >> >> > > >> >> > Done. > >> >> > > >> >> > > >> >> > > >> >> > > >> >> > > https://codereview.appspot.com/6610049/diff/8001/apiclient/discovery.py#newco... > >> >> > apiclient/discovery.py:634: previous_response: The response from > the > >> >> > request for the previous page. (required) > >> >> > On 2012/10/30 18:03:04, jcgregorio_google wrote: > >> >> >> > >> >> >> 80 chars > >> >> > > >> >> > > >> >> > Done. > >> >> > > >> >> > > >> >> > > >> >> > > >> >> > > https://codereview.appspot.com/6610049/diff/8001/apiclient/discovery.py#newco... > >> >> > apiclient/discovery.py:670: def __init__(self, http=None, > >> >> > baseUrl=None, > >> >> > model=None, developerKey=None, > >> >> > On 2012/10/30 18:03:04, jcgregorio_google wrote: > >> >> >> > >> >> >> I don't think any of these needs =None defaults. > >> >> > > >> >> > > >> >> > You are right. I'm not sure why I changed this, and also > accidentally > >> >> > changed the order of developerKey and requestBuilder. > >> >> > > >> >> > > >> >> > > >> >> > > >> >> > > https://codereview.appspot.com/6610049/diff/8001/apiclient/discovery.py#newco... > >> >> > apiclient/discovery.py:739: def set_service_methods(self): > >> >> > On 2012/10/30 18:03:04, jcgregorio_google wrote: > >> >> >> > >> >> >> _set_service_methods > >> >> > > >> >> > > >> >> > Done. > >> >> > > >> >> > https://codereview.appspot.com/6610049/ > >> > > >> > > >> > > >> > > >> > -- > >> > Danny Hermes > >> > Developer Programs Engineer > > > > > > > > > > -- > > Danny Hermes > > Developer Programs Engineer > -- Danny Hermes Developer Programs Engineer
Sign in to reply to this message.
Patch uploaded. There are still some docs that end up different, but they seem to have been patched by human hands. Also, all of a sudden, make docs is failing, complaining about a right tick mark (\u2019) when it tries to right to a file with no codecs. I checked and double-checked and this has not been introduced in my code. Have you guys experienced this? On Tue, Oct 30, 2012 at 12:46 PM, Danny Hermes <dhermes@google.com> wrote: > Yes. That is what I meant. Will update with a patch. > > > On Tue, Oct 30, 2012 at 12:43 PM, Joe Gregorio <jcgregorio@google.com>wrote: > >> On Tue, Oct 30, 2012 at 3:33 PM, Danny Hermes <dhermes@google.com> wrote: >> > It does change things. For example it is removing all the subresources >> from >> > specific API docs. >> > >> > This is likely because the attr is not set on the object so >> object.__dict__ >> > does not get updated and things like dir(service_object) won't list the >> > subresources. >> >> Yeah, definitely want to keep the dir(service_object) behavior unchanged. >> >> > I can set this in a method that will also set it in _attr_cache or just >> > track the attributes that would have ended up in attr_cache via some >> other >> > mechanism. >> >> Or would it be easier to invert the meaning of attr_cache and have it >> just be the name of attributes that will be stripped out of the response >> from >> __getstate__? That way you can also avoid over-riding __getattribute__. >> >> -joe >> >> > >> > What do you think? >> > >> > >> > On Tue, Oct 30, 2012 at 11:37 AM, Joe Gregorio <jcgregorio@google.com> >> > wrote: >> >> >> >> There's a Makefile in the project. Just run: >> >> >> >> $ make docs >> >> >> >> Then look at the diffs in the docs that produces. You probably want to >> >> patch this CL into >> >> another copy of the repository since it will modify the doc files >> >> which you probably don't want >> >> as a part of this CL. >> >> >> >> Thanks, >> >> -joe >> >> >> >> On Tue, Oct 30, 2012 at 2:34 PM, Danny Hermes <dhermes@google.com> >> wrote: >> >> > Nope. How might one do this? >> >> > >> >> > >> >> > On Tue, Oct 30, 2012 at 11:17 AM, Joe Gregorio < >> jcgregorio@google.com> >> >> > wrote: >> >> >> >> >> >> Have you also verified that this doesn't change the output of 'make >> >> >> docs' >> >> >> ? >> >> >> >> >> >> -joe >> >> >> >> >> >> On Tue, Oct 30, 2012 at 2:12 PM, <dhermes@google.com> wrote: >> >> >> > I have already updated the new patch set. >> >> >> > >> >> >> > >> >> >> > >> >> >> > >> >> >> > >> https://codereview.appspot.com/6610049/diff/8001/apiclient/discovery.py >> >> >> > File apiclient/discovery.py (right): >> >> >> > >> >> >> > >> >> >> > >> >> >> > >> https://codereview.appspot.com/6610049/diff/8001/apiclient/discovery.py#newco... >> >> >> > apiclient/discovery.py:384: >> >> >> > On 2012/10/30 18:03:04, jcgregorio_google wrote: >> >> >> >> >> >> >> >> one blank line >> >> >> > >> >> >> > >> >> >> > Done. >> >> >> > >> >> >> > >> >> >> > >> >> >> > >> >> >> > >> https://codereview.appspot.com/6610049/diff/8001/apiclient/discovery.py#newco... >> >> >> > apiclient/discovery.py:634: previous_response: The response from >> the >> >> >> > request for the previous page. (required) >> >> >> > On 2012/10/30 18:03:04, jcgregorio_google wrote: >> >> >> >> >> >> >> >> 80 chars >> >> >> > >> >> >> > >> >> >> > Done. >> >> >> > >> >> >> > >> >> >> > >> >> >> > >> >> >> > >> https://codereview.appspot.com/6610049/diff/8001/apiclient/discovery.py#newco... >> >> >> > apiclient/discovery.py:670: def __init__(self, http=None, >> >> >> > baseUrl=None, >> >> >> > model=None, developerKey=None, >> >> >> > On 2012/10/30 18:03:04, jcgregorio_google wrote: >> >> >> >> >> >> >> >> I don't think any of these needs =None defaults. >> >> >> > >> >> >> > >> >> >> > You are right. I'm not sure why I changed this, and also >> accidentally >> >> >> > changed the order of developerKey and requestBuilder. >> >> >> > >> >> >> > >> >> >> > >> >> >> > >> >> >> > >> https://codereview.appspot.com/6610049/diff/8001/apiclient/discovery.py#newco... >> >> >> > apiclient/discovery.py:739: def set_service_methods(self): >> >> >> > On 2012/10/30 18:03:04, jcgregorio_google wrote: >> >> >> >> >> >> >> >> _set_service_methods >> >> >> > >> >> >> > >> >> >> > Done. >> >> >> > >> >> >> > https://codereview.appspot.com/6610049/ >> >> > >> >> > >> >> > >> >> > >> >> > -- >> >> > Danny Hermes >> >> > Developer Programs Engineer >> > >> > >> > >> > >> > -- >> > Danny Hermes >> > Developer Programs Engineer >> > > > > -- > Danny Hermes > Developer Programs Engineer > -- Danny Hermes Developer Programs Engineer
Sign in to reply to this message.
Yeah, I thought I'd pushed a fix for that, there was an issue with unicode in descriptions. I will look into that. -joe On Tue, Oct 30, 2012 at 4:26 PM, Danny Hermes <dhermes@google.com> wrote: > Patch uploaded. There are still some docs that end up different, but they > seem to have been patched by human hands. > > Also, all of a sudden, make docs is failing, complaining about a right tick > mark (\u2019) when it tries to right to a file with no codecs. I checked and > double-checked and this has not been introduced in my code. > > Have you guys experienced this? > > > On Tue, Oct 30, 2012 at 12:46 PM, Danny Hermes <dhermes@google.com> wrote: >> >> Yes. That is what I meant. Will update with a patch. >> >> >> On Tue, Oct 30, 2012 at 12:43 PM, Joe Gregorio <jcgregorio@google.com> >> wrote: >>> >>> On Tue, Oct 30, 2012 at 3:33 PM, Danny Hermes <dhermes@google.com> wrote: >>> > It does change things. For example it is removing all the subresources >>> > from >>> > specific API docs. >>> > >>> > This is likely because the attr is not set on the object so >>> > object.__dict__ >>> > does not get updated and things like dir(service_object) won't list the >>> > subresources. >>> >>> Yeah, definitely want to keep the dir(service_object) behavior unchanged. >>> >>> > I can set this in a method that will also set it in _attr_cache or just >>> > track the attributes that would have ended up in attr_cache via some >>> > other >>> > mechanism. >>> >>> Or would it be easier to invert the meaning of attr_cache and have it >>> just be the name of attributes that will be stripped out of the response >>> from >>> __getstate__? That way you can also avoid over-riding __getattribute__. >>> >>> -joe >>> >>> > >>> > What do you think? >>> > >>> > >>> > On Tue, Oct 30, 2012 at 11:37 AM, Joe Gregorio <jcgregorio@google.com> >>> > wrote: >>> >> >>> >> There's a Makefile in the project. Just run: >>> >> >>> >> $ make docs >>> >> >>> >> Then look at the diffs in the docs that produces. You probably want to >>> >> patch this CL into >>> >> another copy of the repository since it will modify the doc files >>> >> which you probably don't want >>> >> as a part of this CL. >>> >> >>> >> Thanks, >>> >> -joe >>> >> >>> >> On Tue, Oct 30, 2012 at 2:34 PM, Danny Hermes <dhermes@google.com> >>> >> wrote: >>> >> > Nope. How might one do this? >>> >> > >>> >> > >>> >> > On Tue, Oct 30, 2012 at 11:17 AM, Joe Gregorio >>> >> > <jcgregorio@google.com> >>> >> > wrote: >>> >> >> >>> >> >> Have you also verified that this doesn't change the output of 'make >>> >> >> docs' >>> >> >> ? >>> >> >> >>> >> >> -joe >>> >> >> >>> >> >> On Tue, Oct 30, 2012 at 2:12 PM, <dhermes@google.com> wrote: >>> >> >> > I have already updated the new patch set. >>> >> >> > >>> >> >> > >>> >> >> > >>> >> >> > >>> >> >> > >>> >> >> > https://codereview.appspot.com/6610049/diff/8001/apiclient/discovery.py >>> >> >> > File apiclient/discovery.py (right): >>> >> >> > >>> >> >> > >>> >> >> > >>> >> >> > >>> >> >> > https://codereview.appspot.com/6610049/diff/8001/apiclient/discovery.py#newco... >>> >> >> > apiclient/discovery.py:384: >>> >> >> > On 2012/10/30 18:03:04, jcgregorio_google wrote: >>> >> >> >> >>> >> >> >> one blank line >>> >> >> > >>> >> >> > >>> >> >> > Done. >>> >> >> > >>> >> >> > >>> >> >> > >>> >> >> > >>> >> >> > >>> >> >> > https://codereview.appspot.com/6610049/diff/8001/apiclient/discovery.py#newco... >>> >> >> > apiclient/discovery.py:634: previous_response: The response from >>> >> >> > the >>> >> >> > request for the previous page. (required) >>> >> >> > On 2012/10/30 18:03:04, jcgregorio_google wrote: >>> >> >> >> >>> >> >> >> 80 chars >>> >> >> > >>> >> >> > >>> >> >> > Done. >>> >> >> > >>> >> >> > >>> >> >> > >>> >> >> > >>> >> >> > >>> >> >> > https://codereview.appspot.com/6610049/diff/8001/apiclient/discovery.py#newco... >>> >> >> > apiclient/discovery.py:670: def __init__(self, http=None, >>> >> >> > baseUrl=None, >>> >> >> > model=None, developerKey=None, >>> >> >> > On 2012/10/30 18:03:04, jcgregorio_google wrote: >>> >> >> >> >>> >> >> >> I don't think any of these needs =None defaults. >>> >> >> > >>> >> >> > >>> >> >> > You are right. I'm not sure why I changed this, and also >>> >> >> > accidentally >>> >> >> > changed the order of developerKey and requestBuilder. >>> >> >> > >>> >> >> > >>> >> >> > >>> >> >> > >>> >> >> > >>> >> >> > https://codereview.appspot.com/6610049/diff/8001/apiclient/discovery.py#newco... >>> >> >> > apiclient/discovery.py:739: def set_service_methods(self): >>> >> >> > On 2012/10/30 18:03:04, jcgregorio_google wrote: >>> >> >> >> >>> >> >> >> _set_service_methods >>> >> >> > >>> >> >> > >>> >> >> > Done. >>> >> >> > >>> >> >> > https://codereview.appspot.com/6610049/ >>> >> > >>> >> > >>> >> > >>> >> > >>> >> > -- >>> >> > Danny Hermes >>> >> > Developer Programs Engineer >>> > >>> > >>> > >>> > >>> > -- >>> > Danny Hermes >>> > Developer Programs Engineer >> >> >> >> >> -- >> Danny Hermes >> Developer Programs Engineer > > > > > -- > Danny Hermes > Developer Programs Engineer
Sign in to reply to this message.
Traceback: python describe.py Traceback (most recent call last): File "describe.py", line 346, in <module> document_api(api['name'], api['version']) File "describe.py", line 335, in document_api service, '%s_%s.' % (name, version), discovery, discovery) File "describe.py", line 314, in document_collection_recursive discovery['resources'].get(dname, {})) File "describe.py", line 304, in document_collection_recursive f.write(html) UnicodeEncodeError: 'ascii' codec can't encode character u'\u2019' in position 5677: ordinal not in range(128) make: *** [docs] Error 1 On Tue, Oct 30, 2012 at 1:26 PM, Danny Hermes <dhermes@google.com> wrote: > Patch uploaded. There are still some docs that end up different, but they > seem to have been patched by human hands. > > Also, all of a sudden, make docs is failing, complaining about a right > tick mark (\u2019) when it tries to right to a file with no codecs. I > checked and double-checked and this has not been introduced in my code. > > Have you guys experienced this? > > > On Tue, Oct 30, 2012 at 12:46 PM, Danny Hermes <dhermes@google.com> wrote: > >> Yes. That is what I meant. Will update with a patch. >> >> >> On Tue, Oct 30, 2012 at 12:43 PM, Joe Gregorio <jcgregorio@google.com>wrote: >> >>> On Tue, Oct 30, 2012 at 3:33 PM, Danny Hermes <dhermes@google.com> >>> wrote: >>> > It does change things. For example it is removing all the subresources >>> from >>> > specific API docs. >>> > >>> > This is likely because the attr is not set on the object so >>> object.__dict__ >>> > does not get updated and things like dir(service_object) won't list the >>> > subresources. >>> >>> Yeah, definitely want to keep the dir(service_object) behavior unchanged. >>> >>> > I can set this in a method that will also set it in _attr_cache or just >>> > track the attributes that would have ended up in attr_cache via some >>> other >>> > mechanism. >>> >>> Or would it be easier to invert the meaning of attr_cache and have it >>> just be the name of attributes that will be stripped out of the response >>> from >>> __getstate__? That way you can also avoid over-riding __getattribute__. >>> >>> -joe >>> >>> > >>> > What do you think? >>> > >>> > >>> > On Tue, Oct 30, 2012 at 11:37 AM, Joe Gregorio <jcgregorio@google.com> >>> > wrote: >>> >> >>> >> There's a Makefile in the project. Just run: >>> >> >>> >> $ make docs >>> >> >>> >> Then look at the diffs in the docs that produces. You probably want to >>> >> patch this CL into >>> >> another copy of the repository since it will modify the doc files >>> >> which you probably don't want >>> >> as a part of this CL. >>> >> >>> >> Thanks, >>> >> -joe >>> >> >>> >> On Tue, Oct 30, 2012 at 2:34 PM, Danny Hermes <dhermes@google.com> >>> wrote: >>> >> > Nope. How might one do this? >>> >> > >>> >> > >>> >> > On Tue, Oct 30, 2012 at 11:17 AM, Joe Gregorio < >>> jcgregorio@google.com> >>> >> > wrote: >>> >> >> >>> >> >> Have you also verified that this doesn't change the output of 'make >>> >> >> docs' >>> >> >> ? >>> >> >> >>> >> >> -joe >>> >> >> >>> >> >> On Tue, Oct 30, 2012 at 2:12 PM, <dhermes@google.com> wrote: >>> >> >> > I have already updated the new patch set. >>> >> >> > >>> >> >> > >>> >> >> > >>> >> >> > >>> >> >> > >>> https://codereview.appspot.com/6610049/diff/8001/apiclient/discovery.py >>> >> >> > File apiclient/discovery.py (right): >>> >> >> > >>> >> >> > >>> >> >> > >>> >> >> > >>> https://codereview.appspot.com/6610049/diff/8001/apiclient/discovery.py#newco... >>> >> >> > apiclient/discovery.py:384: >>> >> >> > On 2012/10/30 18:03:04, jcgregorio_google wrote: >>> >> >> >> >>> >> >> >> one blank line >>> >> >> > >>> >> >> > >>> >> >> > Done. >>> >> >> > >>> >> >> > >>> >> >> > >>> >> >> > >>> >> >> > >>> https://codereview.appspot.com/6610049/diff/8001/apiclient/discovery.py#newco... >>> >> >> > apiclient/discovery.py:634: previous_response: The response from >>> the >>> >> >> > request for the previous page. (required) >>> >> >> > On 2012/10/30 18:03:04, jcgregorio_google wrote: >>> >> >> >> >>> >> >> >> 80 chars >>> >> >> > >>> >> >> > >>> >> >> > Done. >>> >> >> > >>> >> >> > >>> >> >> > >>> >> >> > >>> >> >> > >>> https://codereview.appspot.com/6610049/diff/8001/apiclient/discovery.py#newco... >>> >> >> > apiclient/discovery.py:670: def __init__(self, http=None, >>> >> >> > baseUrl=None, >>> >> >> > model=None, developerKey=None, >>> >> >> > On 2012/10/30 18:03:04, jcgregorio_google wrote: >>> >> >> >> >>> >> >> >> I don't think any of these needs =None defaults. >>> >> >> > >>> >> >> > >>> >> >> > You are right. I'm not sure why I changed this, and also >>> accidentally >>> >> >> > changed the order of developerKey and requestBuilder. >>> >> >> > >>> >> >> > >>> >> >> > >>> >> >> > >>> >> >> > >>> https://codereview.appspot.com/6610049/diff/8001/apiclient/discovery.py#newco... >>> >> >> > apiclient/discovery.py:739: def set_service_methods(self): >>> >> >> > On 2012/10/30 18:03:04, jcgregorio_google wrote: >>> >> >> >> >>> >> >> >> _set_service_methods >>> >> >> > >>> >> >> > >>> >> >> > Done. >>> >> >> > >>> >> >> > https://codereview.appspot.com/6610049/ >>> >> > >>> >> > >>> >> > >>> >> > >>> >> > -- >>> >> > Danny Hermes >>> >> > Developer Programs Engineer >>> > >>> > >>> > >>> > >>> > -- >>> > Danny Hermes >>> > Developer Programs Engineer >>> >> >> >> >> -- >> Danny Hermes >> Developer Programs Engineer >> > > > > -- > Danny Hermes > Developer Programs Engineer > -- Danny Hermes Developer Programs Engineer
Sign in to reply to this message.
Can you try building the docs with a working version of make docs? On Tue, Oct 30, 2012 at 1:28 PM, Danny Hermes <dhermes@google.com> wrote: > Traceback: > > python describe.py > Traceback (most recent call last): > File "describe.py", line 346, in <module> > document_api(api['name'], api['version']) > File "describe.py", line 335, in document_api > service, '%s_%s.' % (name, version), discovery, discovery) > File "describe.py", line 314, in document_collection_recursive > discovery['resources'].get(dname, {})) > File "describe.py", line 304, in document_collection_recursive > f.write(html) > UnicodeEncodeError: 'ascii' codec can't encode character u'\u2019' in > position 5677: ordinal not in range(128) > make: *** [docs] Error 1 > > > On Tue, Oct 30, 2012 at 1:26 PM, Danny Hermes <dhermes@google.com> wrote: > >> Patch uploaded. There are still some docs that end up different, but they >> seem to have been patched by human hands. >> >> Also, all of a sudden, make docs is failing, complaining about a right >> tick mark (\u2019) when it tries to right to a file with no codecs. I >> checked and double-checked and this has not been introduced in my code. >> >> Have you guys experienced this? >> >> >> On Tue, Oct 30, 2012 at 12:46 PM, Danny Hermes <dhermes@google.com>wrote: >> >>> Yes. That is what I meant. Will update with a patch. >>> >>> >>> On Tue, Oct 30, 2012 at 12:43 PM, Joe Gregorio <jcgregorio@google.com>wrote: >>> >>>> On Tue, Oct 30, 2012 at 3:33 PM, Danny Hermes <dhermes@google.com> >>>> wrote: >>>> > It does change things. For example it is removing all the >>>> subresources from >>>> > specific API docs. >>>> > >>>> > This is likely because the attr is not set on the object so >>>> object.__dict__ >>>> > does not get updated and things like dir(service_object) won't list >>>> the >>>> > subresources. >>>> >>>> Yeah, definitely want to keep the dir(service_object) behavior >>>> unchanged. >>>> >>>> > I can set this in a method that will also set it in _attr_cache or >>>> just >>>> > track the attributes that would have ended up in attr_cache via some >>>> other >>>> > mechanism. >>>> >>>> Or would it be easier to invert the meaning of attr_cache and have it >>>> just be the name of attributes that will be stripped out of the >>>> response from >>>> __getstate__? That way you can also avoid over-riding __getattribute__. >>>> >>>> -joe >>>> >>>> > >>>> > What do you think? >>>> > >>>> > >>>> > On Tue, Oct 30, 2012 at 11:37 AM, Joe Gregorio <jcgregorio@google.com >>>> > >>>> > wrote: >>>> >> >>>> >> There's a Makefile in the project. Just run: >>>> >> >>>> >> $ make docs >>>> >> >>>> >> Then look at the diffs in the docs that produces. You probably want >>>> to >>>> >> patch this CL into >>>> >> another copy of the repository since it will modify the doc files >>>> >> which you probably don't want >>>> >> as a part of this CL. >>>> >> >>>> >> Thanks, >>>> >> -joe >>>> >> >>>> >> On Tue, Oct 30, 2012 at 2:34 PM, Danny Hermes <dhermes@google.com> >>>> wrote: >>>> >> > Nope. How might one do this? >>>> >> > >>>> >> > >>>> >> > On Tue, Oct 30, 2012 at 11:17 AM, Joe Gregorio < >>>> jcgregorio@google.com> >>>> >> > wrote: >>>> >> >> >>>> >> >> Have you also verified that this doesn't change the output of >>>> 'make >>>> >> >> docs' >>>> >> >> ? >>>> >> >> >>>> >> >> -joe >>>> >> >> >>>> >> >> On Tue, Oct 30, 2012 at 2:12 PM, <dhermes@google.com> wrote: >>>> >> >> > I have already updated the new patch set. >>>> >> >> > >>>> >> >> > >>>> >> >> > >>>> >> >> > >>>> >> >> > >>>> https://codereview.appspot.com/6610049/diff/8001/apiclient/discovery.py >>>> >> >> > File apiclient/discovery.py (right): >>>> >> >> > >>>> >> >> > >>>> >> >> > >>>> >> >> > >>>> https://codereview.appspot.com/6610049/diff/8001/apiclient/discovery.py#newco... >>>> >> >> > apiclient/discovery.py:384: >>>> >> >> > On 2012/10/30 18:03:04, jcgregorio_google wrote: >>>> >> >> >> >>>> >> >> >> one blank line >>>> >> >> > >>>> >> >> > >>>> >> >> > Done. >>>> >> >> > >>>> >> >> > >>>> >> >> > >>>> >> >> > >>>> >> >> > >>>> https://codereview.appspot.com/6610049/diff/8001/apiclient/discovery.py#newco... >>>> >> >> > apiclient/discovery.py:634: previous_response: The response >>>> from the >>>> >> >> > request for the previous page. (required) >>>> >> >> > On 2012/10/30 18:03:04, jcgregorio_google wrote: >>>> >> >> >> >>>> >> >> >> 80 chars >>>> >> >> > >>>> >> >> > >>>> >> >> > Done. >>>> >> >> > >>>> >> >> > >>>> >> >> > >>>> >> >> > >>>> >> >> > >>>> https://codereview.appspot.com/6610049/diff/8001/apiclient/discovery.py#newco... >>>> >> >> > apiclient/discovery.py:670: def __init__(self, http=None, >>>> >> >> > baseUrl=None, >>>> >> >> > model=None, developerKey=None, >>>> >> >> > On 2012/10/30 18:03:04, jcgregorio_google wrote: >>>> >> >> >> >>>> >> >> >> I don't think any of these needs =None defaults. >>>> >> >> > >>>> >> >> > >>>> >> >> > You are right. I'm not sure why I changed this, and also >>>> accidentally >>>> >> >> > changed the order of developerKey and requestBuilder. >>>> >> >> > >>>> >> >> > >>>> >> >> > >>>> >> >> > >>>> >> >> > >>>> https://codereview.appspot.com/6610049/diff/8001/apiclient/discovery.py#newco... >>>> >> >> > apiclient/discovery.py:739: def set_service_methods(self): >>>> >> >> > On 2012/10/30 18:03:04, jcgregorio_google wrote: >>>> >> >> >> >>>> >> >> >> _set_service_methods >>>> >> >> > >>>> >> >> > >>>> >> >> > Done. >>>> >> >> > >>>> >> >> > https://codereview.appspot.com/6610049/ >>>> >> > >>>> >> > >>>> >> > >>>> >> > >>>> >> > -- >>>> >> > Danny Hermes >>>> >> > Developer Programs Engineer >>>> > >>>> > >>>> > >>>> > >>>> > -- >>>> > Danny Hermes >>>> > Developer Programs Engineer >>>> >>> >>> >>> >>> -- >>> Danny Hermes >>> Developer Programs Engineer >>> >> >> >> >> -- >> Danny Hermes >> Developer Programs Engineer >> > > > > -- > Danny Hermes > Developer Programs Engineer > -- Danny Hermes Developer Programs Engineer
Sign in to reply to this message.
The fix for the encoding issue is: diff -r c58ae21bb5e0 describe.py --- a/describe.py Mon Nov 05 08:01:29 2012 -0500 +++ b/describe.py Mon Nov 05 08:03:30 2012 -0500 @@ -301,7 +301,7 @@ html = document_collection(resource, path, root_discovery, discovery) f = open(os.path.join(BASE, path + 'html'), 'w') - f.write(html) + f.write(html.encode('utf-8')) f.close() for name in dir(resource): When I generate all the docs I am still seeing issues. For example, the arguments are missing from list_next(). -joe On Wed, Oct 31, 2012 at 6:30 PM, Danny Hermes <dhermes@google.com> wrote: > Can you try building the docs with a working version of make docs? > > > On Tue, Oct 30, 2012 at 1:28 PM, Danny Hermes <dhermes@google.com> wrote: >> >> Traceback: >> >> python describe.py >> Traceback (most recent call last): >> File "describe.py", line 346, in <module> >> document_api(api['name'], api['version']) >> File "describe.py", line 335, in document_api >> service, '%s_%s.' % (name, version), discovery, discovery) >> File "describe.py", line 314, in document_collection_recursive >> discovery['resources'].get(dname, {})) >> File "describe.py", line 304, in document_collection_recursive >> f.write(html) >> UnicodeEncodeError: 'ascii' codec can't encode character u'\u2019' in >> position 5677: ordinal not in range(128) >> make: *** [docs] Error 1 >> >> >> On Tue, Oct 30, 2012 at 1:26 PM, Danny Hermes <dhermes@google.com> wrote: >>> >>> Patch uploaded. There are still some docs that end up different, but they >>> seem to have been patched by human hands. >>> >>> Also, all of a sudden, make docs is failing, complaining about a right >>> tick mark (\u2019) when it tries to right to a file with no codecs. I >>> checked and double-checked and this has not been introduced in my code. >>> >>> Have you guys experienced this? >>> >>> >>> On Tue, Oct 30, 2012 at 12:46 PM, Danny Hermes <dhermes@google.com> >>> wrote: >>>> >>>> Yes. That is what I meant. Will update with a patch. >>>> >>>> >>>> On Tue, Oct 30, 2012 at 12:43 PM, Joe Gregorio <jcgregorio@google.com> >>>> wrote: >>>>> >>>>> On Tue, Oct 30, 2012 at 3:33 PM, Danny Hermes <dhermes@google.com> >>>>> wrote: >>>>> > It does change things. For example it is removing all the >>>>> > subresources from >>>>> > specific API docs. >>>>> > >>>>> > This is likely because the attr is not set on the object so >>>>> > object.__dict__ >>>>> > does not get updated and things like dir(service_object) won't list >>>>> > the >>>>> > subresources. >>>>> >>>>> Yeah, definitely want to keep the dir(service_object) behavior >>>>> unchanged. >>>>> >>>>> > I can set this in a method that will also set it in _attr_cache or >>>>> > just >>>>> > track the attributes that would have ended up in attr_cache via some >>>>> > other >>>>> > mechanism. >>>>> >>>>> Or would it be easier to invert the meaning of attr_cache and have it >>>>> just be the name of attributes that will be stripped out of the >>>>> response from >>>>> __getstate__? That way you can also avoid over-riding __getattribute__. >>>>> >>>>> -joe >>>>> >>>>> > >>>>> > What do you think? >>>>> > >>>>> > >>>>> > On Tue, Oct 30, 2012 at 11:37 AM, Joe Gregorio >>>>> > <jcgregorio@google.com> >>>>> > wrote: >>>>> >> >>>>> >> There's a Makefile in the project. Just run: >>>>> >> >>>>> >> $ make docs >>>>> >> >>>>> >> Then look at the diffs in the docs that produces. You probably want >>>>> >> to >>>>> >> patch this CL into >>>>> >> another copy of the repository since it will modify the doc files >>>>> >> which you probably don't want >>>>> >> as a part of this CL. >>>>> >> >>>>> >> Thanks, >>>>> >> -joe >>>>> >> >>>>> >> On Tue, Oct 30, 2012 at 2:34 PM, Danny Hermes <dhermes@google.com> >>>>> >> wrote: >>>>> >> > Nope. How might one do this? >>>>> >> > >>>>> >> > >>>>> >> > On Tue, Oct 30, 2012 at 11:17 AM, Joe Gregorio >>>>> >> > <jcgregorio@google.com> >>>>> >> > wrote: >>>>> >> >> >>>>> >> >> Have you also verified that this doesn't change the output of >>>>> >> >> 'make >>>>> >> >> docs' >>>>> >> >> ? >>>>> >> >> >>>>> >> >> -joe >>>>> >> >> >>>>> >> >> On Tue, Oct 30, 2012 at 2:12 PM, <dhermes@google.com> wrote: >>>>> >> >> > I have already updated the new patch set. >>>>> >> >> > >>>>> >> >> > >>>>> >> >> > >>>>> >> >> > >>>>> >> >> > >>>>> >> >> > https://codereview.appspot.com/6610049/diff/8001/apiclient/discovery.py >>>>> >> >> > File apiclient/discovery.py (right): >>>>> >> >> > >>>>> >> >> > >>>>> >> >> > >>>>> >> >> > >>>>> >> >> > https://codereview.appspot.com/6610049/diff/8001/apiclient/discovery.py#newco... >>>>> >> >> > apiclient/discovery.py:384: >>>>> >> >> > On 2012/10/30 18:03:04, jcgregorio_google wrote: >>>>> >> >> >> >>>>> >> >> >> one blank line >>>>> >> >> > >>>>> >> >> > >>>>> >> >> > Done. >>>>> >> >> > >>>>> >> >> > >>>>> >> >> > >>>>> >> >> > >>>>> >> >> > >>>>> >> >> > https://codereview.appspot.com/6610049/diff/8001/apiclient/discovery.py#newco... >>>>> >> >> > apiclient/discovery.py:634: previous_response: The response >>>>> >> >> > from the >>>>> >> >> > request for the previous page. (required) >>>>> >> >> > On 2012/10/30 18:03:04, jcgregorio_google wrote: >>>>> >> >> >> >>>>> >> >> >> 80 chars >>>>> >> >> > >>>>> >> >> > >>>>> >> >> > Done. >>>>> >> >> > >>>>> >> >> > >>>>> >> >> > >>>>> >> >> > >>>>> >> >> > >>>>> >> >> > https://codereview.appspot.com/6610049/diff/8001/apiclient/discovery.py#newco... >>>>> >> >> > apiclient/discovery.py:670: def __init__(self, http=None, >>>>> >> >> > baseUrl=None, >>>>> >> >> > model=None, developerKey=None, >>>>> >> >> > On 2012/10/30 18:03:04, jcgregorio_google wrote: >>>>> >> >> >> >>>>> >> >> >> I don't think any of these needs =None defaults. >>>>> >> >> > >>>>> >> >> > >>>>> >> >> > You are right. I'm not sure why I changed this, and also >>>>> >> >> > accidentally >>>>> >> >> > changed the order of developerKey and requestBuilder. >>>>> >> >> > >>>>> >> >> > >>>>> >> >> > >>>>> >> >> > >>>>> >> >> > >>>>> >> >> > https://codereview.appspot.com/6610049/diff/8001/apiclient/discovery.py#newco... >>>>> >> >> > apiclient/discovery.py:739: def set_service_methods(self): >>>>> >> >> > On 2012/10/30 18:03:04, jcgregorio_google wrote: >>>>> >> >> >> >>>>> >> >> >> _set_service_methods >>>>> >> >> > >>>>> >> >> > >>>>> >> >> > Done. >>>>> >> >> > >>>>> >> >> > https://codereview.appspot.com/6610049/ >>>>> >> > >>>>> >> > >>>>> >> > >>>>> >> > >>>>> >> > -- >>>>> >> > Danny Hermes >>>>> >> > Developer Programs Engineer >>>>> > >>>>> > >>>>> > >>>>> > >>>>> > -- >>>>> > Danny Hermes >>>>> > Developer Programs Engineer >>>> >>>> >>>> >>>> >>>> -- >>>> Danny Hermes >>>> Developer Programs Engineer >>> >>> >>> >>> >>> -- >>> Danny Hermes >>> Developer Programs Engineer >> >> >> >> >> -- >> Danny Hermes >> Developer Programs Engineer > > > > > -- > Danny Hermes > Developer Programs Engineer
Sign in to reply to this message.
Thanks. I figured out the issue. Your parser in describe.method_params was assuming 0 padding in docstrings, and I was indenting so it looked like a docstring. For this commit, let's leave the parser alone, but then fix it for the next go around. Since I aligned the triple quotes, all of the dyn html files have a closing pre tag with 2 extra spaces because our nextMethod definitions occur at different indent levels.
Sign in to reply to this message.
LGTM On 2012/11/05 17:47:46, dhermes wrote: > Thanks. I figured out the issue. Your parser in describe.method_params was > assuming 0 padding in docstrings, and I was indenting so it looked like a > docstring. > > For this commit, let's leave the parser alone, but then fix it for the next go > around. > > Since I aligned the triple quotes, all of the dyn html files have a closing pre > tag with 2 extra spaces because our nextMethod definitions occur at different > indent levels.
Sign in to reply to this message.
Committed in http://code.google.com/p/google-api-python-client/source/detail?r=5d8ad3d7d2b... That was an epic refactoring, thanks for sticking with it. -joe On 2012/11/06 17:04:46, jcgregorio_google wrote: > LGTM > > On 2012/11/05 17:47:46, dhermes wrote: > > Thanks. I figured out the issue. Your parser in describe.method_params was > > assuming 0 padding in docstrings, and I was indenting so it looked like a > > docstring. > > > > For this commit, let's leave the parser alone, but then fix it for the next go > > around. > > > > Since I aligned the triple quotes, all of the dyn html files have a closing > pre > > tag with 2 extra spaces because our nextMethod definitions occur at different > > indent levels.
Sign in to reply to this message.
Woo hoo!! On Tue, Nov 20, 2012 at 11:31 AM, <jcgregorio@google.com> wrote: > Committed in > http://code.google.com/p/**google-api-python-client/**source/detail?r=** > 5d8ad3d7d2b6cf52d1df1f4a49737e**776708e446<http://code.google.com/p/google-api-python-client/source/detail?r=5d8ad3d7d2b6cf52d1df1f4a49737e776708e446> > > That was an epic refactoring, thanks for sticking with it. > > -joe > > > On 2012/11/06 17:04:46, jcgregorio_google wrote: > >> LGTM >> > > On 2012/11/05 17:47:46, dhermes wrote: >> > Thanks. I figured out the issue. Your parser in >> > describe.method_params was > >> > assuming 0 padding in docstrings, and I was indenting so it looked >> > like a > >> > docstring. >> > >> > For this commit, let's leave the parser alone, but then fix it for >> > the next go > >> > around. >> > >> > Since I aligned the triple quotes, all of the dyn html files have a >> > closing > >> pre >> > tag with 2 extra spaces because our nextMethod definitions occur at >> > different > >> > indent levels. >> > > > https://codereview.appspot.**com/6610049/<https://codereview.appspot.com/6610... > -- Danny Hermes Developer Programs Engineer
Sign in to reply to this message.
+1 On Tue, Nov 20, 2012 at 11:31 AM, Danny Hermes <dhermes@google.com> wrote: > Woo hoo!! > > > On Tue, Nov 20, 2012 at 11:31 AM, <jcgregorio@google.com> wrote: > >> Committed in >> http://code.google.com/p/**google-api-python-client/**source/detail?r=** >> 5d8ad3d7d2b6cf52d1df1f4a49737e**776708e446<http://code.google.com/p/google-api-python-client/source/detail?r=5d8ad3d7d2b6cf52d1df1f4a49737e776708e446> >> >> That was an epic refactoring, thanks for sticking with it. >> >> -joe >> >> >> On 2012/11/06 17:04:46, jcgregorio_google wrote: >> >>> LGTM >>> >> >> On 2012/11/05 17:47:46, dhermes wrote: >>> > Thanks. I figured out the issue. Your parser in >>> >> describe.method_params was >> >>> > assuming 0 padding in docstrings, and I was indenting so it looked >>> >> like a >> >>> > docstring. >>> > >>> > For this commit, let's leave the parser alone, but then fix it for >>> >> the next go >> >>> > around. >>> > >>> > Since I aligned the triple quotes, all of the dyn html files have a >>> >> closing >> >>> pre >>> > tag with 2 extra spaces because our nextMethod definitions occur at >>> >> different >> >>> > indent levels. >>> >> >> >> https://codereview.appspot.**com/6610049/<https://codereview.appspot.com/6610... >> > > > > -- > Danny Hermes > Developer Programs Engineer > -- Ali Afshar | www.googplus.org/ali | Google Developer Relations
Sign in to reply to this message.
|