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.

Sign up for GitHub

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

Jump to bottom

Docstrings and autodocs for API reference #17

Merged
merged 4 commits into from Aug 21, 2015
Merged

Docstrings and autodocs for API reference #17

merged 4 commits into from Aug 21, 2015

Conversation

eliasdorneles
Copy link
Member

Hey, fellows!

Here is a change to use docstrings + autodocs for the API reference, making it easier to keep it in sync with the code.
This fixes #16

Does this look good?

eliasdorneles
eliasdorneles reviewed Aug 20, 2015
View reviewed changes
docs/conf.py
@@ -111,7 +111,7 @@

# The theme to use for HTML and HTML Help pages. See the documentation for
# a list of builtin themes.
html_theme = 'default'
html_theme = 'sphinx_rtd_theme'
Copy link
Member Author

Choose a reason for hiding this comment

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

I've took the opportunity to also setup RTD theme on local build of docs. :)

@codecov-io
Copy link

Current coverage is 100.00%

Merging #17 into master will not affect coverage as of 524ea3e

@@            master     #17   diff @@
======================================
  Files            4       4       
  Stmts          194     194       
  Branches        28      28       
  Methods          0       0       
======================================
  Hit            194     194       
  Partial          0       0       
  Missed           0       0

Review entire Coverage Diff as of 524ea3e

Powered by Codecov. Updated on successful CI builds.

kmike
kmike reviewed Aug 21, 2015
View reviewed changes
parsel/selector.py
self.namespaces[prefix] = uri

def remove_namespaces(self):
"""
Remove all namespaces, allowing to traverse the document using
namespace-less xpaths. See example below.
Copy link
Member

Choose a reason for hiding this comment

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

"See example below" doesn't make much sense in a docstring. What about See :ref:Removing namespaces <...>?

Copy link
Member Author

Choose a reason for hiding this comment

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

good catch, thanks @kmike :)

kmike
kmike reviewed Aug 21, 2015
View reviewed changes
parsel/selector.py
:class:`Selector` allows you to select parts of an XML or HTML text using CSS
or XPath expressions and extract data from it.

``text`` is utf-8 encoded text (unicode object in Python 3 or str in Python 3)
Copy link
Member

Choose a reason for hiding this comment

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

This statement is contradictory: utf8-encoded text is bytes by definition, it is an opposite of Py2 unicode / Py 3 str.

Copy link
Member Author

Choose a reason for hiding this comment

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

Right! So, I'll just put the same thing I used for the usage docs:

``text`` is an unicode object in Python 2 or an str object in Python 3

sounds good?

Copy link
Member

Choose a reason for hiding this comment

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

yep! a minor grammar fix: a str object

Copy link
Member

Choose a reason for hiding this comment

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

and a unicode

@kmike
Copy link
Member

kmike commented Aug 21, 2015

👍

kmike added a commit that referenced this pull request Aug 21, 2015
@kmike
Merge pull request #17 from scrapy/doc-fixes
7ad79ba 
Docstrings and autodocs for API reference
@kmike kmike merged commit 7ad79ba into master Aug 21, 2015
@kmike kmike deleted the doc-fixes branch August 21, 2015 18:58
@dangra
Copy link
Member

dangra commented Aug 22, 2015

:shipit:

@eliasdorneles
Copy link
Member Author

@dangra should we try to add the custom xpath funcs before 1.0?

@dangra
Copy link
Member

dangra commented Aug 22, 2015

No, it's OK to release with what we have
El ago. 22, 2015 15:48, "Elias Dorneles" notifications@github.com
escribió:

@dangra https://github.com/dangra should we try to add the custom xpath
funcs before 1.0?

—
Reply to this email directly or view it on GitHub
#17 (comment).

@eliasdorneles
Copy link
Member Author

Alright, can I go ahead update HISTORY and release 1.0?

@eliasdorneles
Copy link
Member Author

@dangra HISTORY updated, green sign for bumpversion major ? :)

@dangra
Copy link
Member

dangra commented Aug 22, 2015

Go ahead !

@dangra
Copy link
Member

dangra commented Aug 22, 2015

There are some use of unnecessary type='html' usage, i.e.: https://github.com/scrapy/parsel/blame/master/docs/usage.rst#L354

@eliasdorneles
Copy link
Member Author

alright, I'll fix that first! =)

@eliasdorneles
Copy link
Member Author

done, 1.0 released! :)
I had to update the PyPI long description manually after the release, because I messed up the date in HISTORY (see: 2143483)

phew!
thanks for all the help fellows, going to chill out a bit now! 🍷 🍸

@dangra
Copy link
Member

dangra commented Aug 23, 2015

woohoo! 🎆

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Move API docs to autodocs/docstrings
4 participants
@eliasdorneles @codecov-io @kmike @dangra