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

Doc for objects that inherit from BaseCoordinateFrame is messy #7458

Open
dstansby opened this issue May 12, 2018 · 15 comments
Open

Doc for objects that inherit from BaseCoordinateFrame is messy #7458

dstansby opened this issue May 12, 2018 · 15 comments
Labels
coordinates Docs Effort-low good first issue Issues that are well-suited for new contributors Package-novice

Comments

@dstansby
Copy link
Contributor

e.g. see this page: http://docs.sunpy.org/en/stable/api/sunpy.coordinates.frames.HeliographicStonyhurst.html#sunpy.coordinates.frames.HeliographicStonyhurst and scroll down to "Attributes summary"

Because BaseCoordinateFrame has a few class attributes, these get automatically documented (without any docstrings) when documenting an inheriting class. It would be nice to make these disappear somehow - maybe converting them to properties, and making the class attributes hidden by putting an underscore before their names?

@pllim
Copy link
Member

pllim commented May 14, 2018

Shouldn't this be a sunpy issue? They can hide whatever they want using "exclude members" directive, no?

@dstansby
Copy link
Contributor Author

Maybe the attributes should be documented in astropy too?

@pllim
Copy link
Member

pllim commented May 14, 2018

Sure, documenting the attributes is a good idea. However, changing the code for the sake of documentation is probably not (unless there is a very good reason to do so). Thank you for bringing this to our attention!

@pllim
Copy link
Member

pllim commented May 14, 2018

And last but not the least, you are welcome to contribute!

@dstansby
Copy link
Contributor Author

👍 to documenting the properties - if I have time I will have a look, but busy with lots of other stuff right now

@pllim pllim added the good first issue Issues that are well-suited for new contributors label May 23, 2018
@astrofrog astrofrog added sprint and removed sprint labels May 23, 2018
@b1quint
Copy link

b1quint commented Dec 7, 2018

May I contribute by addressing this issue?

@pllim
Copy link
Member

pllim commented Dec 7, 2018

@b1quint , of course. PR is welcome!

@Himanshu-Garg
Copy link

Anyone doing this?
Or should i start with it, as there is no PR mentioning this issue and last comment is 3 months old.

@b1quint
Copy link

b1quint commented Mar 22, 2019

Yeah, I was trying to help on that but I could not understand what the code was doing. I should have reported on that. If you believe you can do it, go for it!

@Himanshu-Garg
Copy link

Sure, will try

@astrojuanlu
Copy link
Member

Still an issue:

Screenshot_2020-10-04 HeliographicStonyhurst

@astrojuanlu astrojuanlu added the Hacktoberfest Hacktoberfest annual event label Oct 4, 2020
@astrojuanlu
Copy link
Member

To be more precise, some of the required info is here:

* `default_representation`
A subclass of `~astropy.coordinates.BaseRepresentation` that will be
treated as the default representation of this frame. This is the
representation assumed by default when the frame is created.
* `default_differential`
A subclass of `~astropy.coordinates.BaseDifferential` that will be
treated as the default differential class of this frame. This is the
differential class assumed by default when the frame is created.
* `~astropy.coordinates.Attribute` class attributes
Frame attributes such as ``FK4.equinox`` or ``FK4.obstime`` are defined
using a descriptor class. See the narrative documentation or
built-in classes code for details.
* `frame_specific_representation_info`
A dictionary mapping the name or class of a representation to a list of
`~astropy.coordinates.RepresentationMapping` objects that tell what
names and default units should be used on this frame for the components
of that representation.

I suppose that writing a proper docstring for BaseCoordinateFrame where these properties are documented in the NumPy style (as the rest of Astropy docstrings) should be enough, but I haven't checked.

@Shaheer-Ahmd
Copy link
Contributor

Is this issue still relevant or is it resolved? Should I work on it?

@pllim
Copy link
Member

pllim commented Aug 29, 2023

@dstansby or @ayshih , do you know if this issue is still relevant?

@pllim pllim removed the Hacktoberfest Hacktoberfest annual event label Aug 29, 2023
@ayshih
Copy link
Contributor

ayshih commented Aug 29, 2023

Docstrings for the class properties (e.g., default_differential) were already added in #12005. (Amusingly, I have zero recollection of writing that PR...)

The class variables (name and frame_attributes) are just variables, but we could make them properties if we want to add docstrings. Users shouldn't be modifying those anyway.

As an aside, even if the above is fixed, any frame attribute (e.g., obstime) currently does not have a docstring, so a way would need to be provided to set a docstring for the descriptor.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
coordinates Docs Effort-low good first issue Issues that are well-suited for new contributors Package-novice
Projects
None yet
Development

No branches or pull requests

8 participants