Feedback

[jfyu]

• gslides version: (you need to do a release and tags for the version to be consistent)
• Python version: 3.7
• Operating System: Linux

docs

• Probably should bold or make it more obvious which API we need, right now it's in the notes which actually happens to be smaller font than regular doc
• getting auth is confusing, it would appear to me that I should/would have to enable the scope of the app for the required API? (wasn't obvious till the code), also I didn't realize it had to be a web application I downloaded a desktop application
• https://michael-gracie.github.io/gslides/basic.html (jupyter notebook link broken)
• also wasn't obvious that I had to pick myself as a tester but that might be a Google doc issue

other feedback

• Picky but it's very strange that the child class of something called a Series should be things like Line/Scatter/Area etc....wouldn't these come under something like Plot or Chart or something? If something inherited from a Series I'd expect series manipulation (I don't know what that would be to be honest but feels like the children classes are mostly plotting kind..)
• Is there a particular reason why one must call the execute method? This comes back to the naming issue. If you name something CreateSpreadsheet I would expect the class to create as opposed to having to do an execute function. Is there a particular reason why the execute has to be called explicitly?
• Also, just starring at https://github.com/michael-gracie/gslides/blob/main/gslides/sheetsframe.py, looks like a lot of things are repeating itself, you could definite do with a super class and inherit, at least the CreateSpreadsheet and CreateSheet
• I second Kun's addSlide/rmSlide idea, it feels so much more natural than calling a construction method everytime. A follow up to this is that it's actually kind of strange that everything is a class. It seems that the only method I would ever call on every class is execute, which goes back to the previous point. It definitely feels like those Line/Area/Scatter can be methods as opposed to classes
• personal habit but maybe not zero logging? This is hard to debug if any bug.

Overall though the functionality is good and it's pretty cool. I haven't seen a similar package in pypi so this might be useful to some people (if their internal organisation opens the Google API). Advanced usage I could think of would be porting the presentation back into python and manipulate the graphs, because currently this is only going one way (data -> plot). What if I want to get the data back and do something else?

[michael-gracie]

• Picky but it's very strange that the child class of something called a Series should be things like Line/Scatter/Area etc....wouldn't these come under something like Plot or Chart or something? If something inherited from a Series I'd expect series manipulation (I don't know what that would be to be honest but feels like the children classes are mostly plotting kind..)

I think that goes back to Kun's comment about aligning naming. Series is a naming that google uses but I am not consistent across the package

[michael-gracie]

• I second Kun's addSlide/rmSlide idea, it feels so much more natural than calling a construction method everytime. A follow up to this is that it's actually kind of strange that everything is a class. It seems that the only method I would ever call on every class is execute, which goes back to the previous point. It definitely feels like those Line/Area/Scatter can be methods as opposed to classes

Definitely will do the full refactor to make the class into objects

[michael-gracie]

• personal habit but maybe not zero logging? This is hard to debug if any bug.

Good point. The one valuable piece of information that would make sense logging is the json blob that is being passed to the Google API