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

Update docstrings/pydoc/help (#170) #438

Merged
merged 15 commits into from
Oct 30, 2017

Conversation

gabrielkrell
Copy link
Contributor

Add docstrings in main package, helpers. Now things will show up nicely when people show documentation in their editor, use help, or use pydoc to browse the documentation in HTML form.

Downside: much of the Mail helper docs are copied from the v3 docs, which isn't great for maintainability. I think it'd be too unapproachable just to say "read the docs", and linking docs updates to these docstrings would be a lot of work for the rewards. Maybe a future automation candidate though :)

Example output:

Pop-up docs in editors:
image

pydoc HTML:
image

Package-level help:

>>> import sendgrid
>>> help(sendgrid)
Help on package sendgrid:

NAME
    sendgrid

DESCRIPTION
    This library allows you to quickly and easily use the SendGrid Web API v3 via
    Python.

    For more information on this library, see the README on Github.
        https://github.com/sendgrid/sendgrid-python
    For more information on the SendGrid v3 API, see the v3 docs:
        http://sendgrid.com/docs/API_Reference/api_v3.html
    For the user guide, code examples, and more, visit the main docs page:
        http://sendgrid.com/docs/index.html

    Available subpackages
    ---------------------
    helpers
        Modules to help with common tasks.

PACKAGE CONTENTS
    helpers (package)
    sendgrid
    version
-- More  --

And individual classes:

>>> from sendgrid.helpers.mail import Mail
>>> help(Mail)
Help on class Mail in module sendgrid.helpers.mail.mail:

class Mail(builtins.object)
 |  A request to be sent with the SendGrid v3 Mail Send API (v3/mail/send).
 |
 |  Use get() to get the request body.
 |
 |  Methods defined here:
-- More  --

Add pydoc for package.  I don't want to duplicate the REAME here, so directing users to it instead.

HTTPS doesn't get highlighted for some reason, so HTTP link used.
Some copied from config.yml.
These are sphinx style, but it takes up a lot of space.
Still a few to go.
Apparently Sphinx can do list(type) or list[type].  If we ever use  it this'll make things more clear.
Add docstrings for methods, properties. Typo fix in Mail.send_at
Whew, almost done.
These don't really add anything over the class docstrings, but they're also the thing that most people will look at.
Can't use these in Python 2 because we haven't declared an encoding.  Oops.
@thinkingserious thinkingserious added the status: code review request requesting a community code review or review from Twilio label Oct 25, 2017
@codecov-io
Copy link

codecov-io commented Oct 25, 2017

Codecov Report

Merging #438 into master will decrease coverage by 0.08%.
The diff coverage is 0%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #438      +/-   ##
==========================================
- Coverage      83%   82.92%   -0.09%     
==========================================
  Files          30       30              
  Lines        1024     1025       +1     
  Branches      160      161       +1     
==========================================
  Hits          850      850              
- Misses         84       85       +1     
  Partials       90       90
Impacted Files Coverage Δ
sendgrid/helpers/mail/section.py 85.71% <ø> (ø) ⬆️
sendgrid/helpers/mail/mail_settings.py 87.5% <ø> (ø) ⬆️
sendgrid/helpers/mail/bcc_settings.py 83.33% <ø> (ø) ⬆️
sendgrid/helpers/inbound/__init__.py 100% <ø> (ø) ⬆️
sendgrid/helpers/mail/asm.py 73.91% <ø> (ø) ⬆️
sendgrid/helpers/mail/sandbox_mode.py 85.71% <ø> (ø) ⬆️
sendgrid/helpers/mail/mail.py 97.51% <ø> (ø) ⬆️
sendgrid/helpers/mail/header.py 85.71% <ø> (ø) ⬆️
sendgrid/helpers/mail/tracking_settings.py 96.96% <ø> (ø) ⬆️
sendgrid/helpers/inbound/app.py 73.91% <ø> (ø) ⬆️
... and 18 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 35cc980...c64d7fd. Read the comment docs.

@mbernier
Copy link
Contributor

You, sir, are an animal.

print(response.headers)
print(response.body)

if __name__ == '__main__':
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This isn't a doc string! Looks like this snuck in?

Copy link
Contributor Author

@gabrielkrell gabrielkrell Oct 28, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oops, forgot to document that. pydoc imports files to get their docstrings, so the unconditional code there executes and causes an error since it isn't in the right context. I added an escape so that it just imports and doesn't attempt to do anything unless it's being run directly or from the command line.

Shuffle docs around to fit the new Mail helper structure.  Hopefully I didn't miss anything; due to the amount of code getting moved around. I'll check the diff against master and then revert if needed.
Clear up some straggling stray whitespace, etc.
Put TrackingSettings in the right place.
@gabrielkrell
Copy link
Contributor Author

I wish codeclimate didn't look at docstrings - they're repetitive by nature, resulting in a lot of errors. @mbernier, resolved merge conflicts from #472.

@mbernier
Copy link
Contributor

I think we can configure the docstring problem at codeclimate, it is running on an extremely simplified config - https://github.com/sendgrid/sendgrid-python/blob/master/.codeclimate.yml This can be updated, but I didn't look into it yet.

@mbernier
Copy link
Contributor

I am not sure if this is related to this PR, but I am getting an error in Travis after it runs tests:

  File "/home/travis/build/sendgrid/sendgrid-python/test/test_mail.py", line 410, in test_kitchenSink

more info

@gabrielkrell
Copy link
Contributor Author

gabrielkrell commented Oct 29, 2017

I made an issue (#483) for the Code Climate thing (not a big deal IMO, maybe a good last-minute Hacktoberfest) and will look at the Travis stuff.

I hate moving around big chunks of code like this; I wish git was more intelligent moving things between files.
@mbernier
Copy link
Contributor

@gabrielkrell let me know when you're g2g on this one :)

@gabrielkrell
Copy link
Contributor Author

@mbernier I think we're all set. Excited to see this in master.

@mbernier mbernier merged commit f1933ff into sendgrid:master Oct 30, 2017
@thinkingserious
Copy link
Contributor

Hello @gabrielkrell,

Thanks again for the PR!

We want to show our appreciation by sending you some swag. Could you please fill out this form so we can send it to you? Thanks!

Team SendGrid DX

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
difficulty: hard fix is hard in difficulty status: code review request requesting a community code review or review from Twilio
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants