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

Add parameter doc to sunpy/sun/sun.py #2369

Merged
merged 5 commits into from
Dec 17, 2017
Merged

Conversation

dstansby
Copy link
Member

There is a lot of repetition here because all the methods take the same argument - I wasn't sure how to avoid just copy and pasting the same bit of text, so that's what I've done. If anyone else knows a better way to do this please let me know!

Copy link
Member

@dpshelio dpshelio left a comment

Choose a reason for hiding this comment

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

In pandas I've seen they use a dictionary with docstrings that they append to the functions that need it. Check this https://github.com/pandas-dev/pandas/blob/master/pandas/core/generic.py#L477 Maybe something could be done in here in a similar fashion, but I think that requires some extra thinking.

@Cadair
Copy link
Member

Cadair commented Dec 13, 2017

I haven't tested it but I think this would work:

PARAMETER_DOCS = """
Parameters
---------------
...
""""

def add_parameter_docs(f):
    f.__doc__+= PARAMETER_DOCS
    return f

@add_parameter_docs
def sun_location(t='now'):
    """
    This does a thing.

    """

@nabobalis
Copy link
Contributor

nabobalis commented Dec 13, 2017

I just did that locally.

It looks a bit off but I can't figure out why.

screenshot from 2017-12-13 13-54-44
vs
screenshot from 2017-12-13 13-54-49

Edit: Fixed it but it created a warning in the doc build. #ohwell

sunpy/sun/sun.py Outdated
def add_parameter_docs(f):
f.__doc__ += PARAMETER_DOCS
if isinstance(f.__doc__, str):
Copy link
Member

Choose a reason for hiding this comment

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

this should be six.string_types

Copy link
Contributor

Choose a reason for hiding this comment

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

OH YEAH IN THIS CRAZY WORLD

@Cadair Cadair added Documentation Affects the documentation Merge When CI Passes Hit that merge button when it's all green! sun Affects the sun submodule [DocFix] Affects Release An issue/bug that affects a released version (use a version label too) labels Dec 13, 2017
@nabobalis
Copy link
Contributor

We should move the decorator somewhere else just incase we need it for the future.

@Cadair
Copy link
Member

Cadair commented Dec 13, 2017

Yeah, we should refactor it into utils and let it take arguments. Maybe a prepend and append argument to the decorator?

We should merge this then open a separate issue for it.

@Cadair
Copy link
Member

Cadair commented Dec 13, 2017

We could also make it do a .format on the docstring if a dictionary is provided as an argument to the decorator.

@nabobalis
Copy link
Contributor

Looks like I messed up.

/home/travis/build/sunpy/sunpy/build/lib.linux-x86_64-3.6/sunpy/sun/sun.py:docstring of sunpy.sun.heliographic_solar_center:6: SEVERE: Unexpected section title.

Parameters
----------
/home/travis/build/sunpy/sunpy/build/lib.linux-x86_64-3.6/sunpy/sun/sun.py:docstring of sunpy.sun.solar_north:6: SEVERE: Unexpected section title.

Parameters
----------
/home/travis/build/sunpy/sunpy/build/lib.linux-x86_64-3.6/sunpy/sun/sun.py:docstring of sunpy.sun.sunearth_distance:6: ERROR: Unexpected indentation.
/home/travis/build/sunpy/sunpy/build/lib.linux-x86_64-3.6/sunpy/sun/sun.py:docstring of sunpy.sun.sunearth_distance:7: SEVERE: Unexpected section title.

I will fix these soon(TM)

@nabobalis
Copy link
Contributor

Doc builds without problems now.

@nabobalis nabobalis merged commit e445077 into sunpy:master Dec 17, 2017
@nabobalis nabobalis mentioned this pull request Dec 17, 2017
@dstansby dstansby deleted the sun-param-doc branch December 17, 2017 13:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Affects Release An issue/bug that affects a released version (use a version label too) Documentation Affects the documentation Merge When CI Passes Hit that merge button when it's all green! sun Affects the sun submodule
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants