jbms / sphinx-immaterial

Adaptation of the popular mkdocs-material material design theme to the sphinx documentation system
https://jbms.github.io/sphinx-immaterial/
Other
200 stars 32 forks source link

Distinguish between class and instance methods/properties #102

Open mhostetter opened 2 years ago

mhostetter commented 2 years ago

This is a feature request. I think it might be nice to distinguish between class methods and methods as well as class properties and properties with the colored "M" and "P" icons. Perhaps a "CM" for class method and "CP" for class property? Maybe even a slightly different shade of the color?

Here I have a class with several different class methods, methods, and class properties. Here is a screen shot.

image

With the new python-apigen it will be easier to categorize different types of methods, but I still think adding a little more differentiation could be visually helpful.

2bndy5 commented 2 years ago

Perhaps a "CM" for class method

This can already be done via toc_icon_text config option:

object_description_options = [
    ("py:classmethod", dict(toc_icon_text="CM")),
]

"CP" for class property

This seems like a limitation inherited by sphinx' python domain. Python properties (per config defaults) are not distinguished in this way.

Maybe even a slightly different shade of the color

I agree the 4 options accepted by toc_icon_class seem a bit limited for dense APIs. I'd suggest a CSS solution, but there isn't a CSS class that would narrow down the ideal CSS selector enough. Maybe we could append an additional CSS class based on the domain identifier, like objinfo-icon__py-classmethod

2bndy5 commented 2 years ago

There was also some discussion a while back (in #22) about formatting the signature in the page content better.

For example:

classmethod Elements(dtype: DTypeLike | None = None) → FieldArray

would be

@classmethod
Elements(dtype: DTypeLike | None = None) → FieldArray

as it would be written in python source code.

2bndy5 commented 2 years ago

I agree the 4 options accepted by toc_icon_class seem a bit limited ... Maybe we could append an additional CSS class based on the domain identifier, like objinfo-icon__py-classmethod

This doesn't seem noted in the docs because it may lead to undefined behavior, but toc_icon_class string value is appended to the CSS classes. https://github.com/jbms/sphinx-immaterial/blob/bd1e3595e6e114a3bb6c20369112e1533542bce1/sphinx_immaterial/nav_adapt.py#L304-L309 However, this still doesn't offer enough distinction as requested here.

mhostetter commented 2 years ago

This can already be done via toc_icon_text config option:

Oh, wow! Great! I didn't know about that.

I just tried using it, however I'm having some issues. I'm building my project using main bd1e3595e6e114a3bb6c20369112e1533542bce1 of the theme.

object_description_options = [
    ("py:function", dict(include_fields_in_toc=False)),
    ("py:method", dict(include_fields_in_toc=False)),
    ("py:classmethod", dict(toc_icon_text="CM")),
]

I have these settings, however it seems I can't suppress the fields in methods or modify the classmethod icon text. Here's a screenshot. Am I doing something wrong? Notice, FLFSR.Taps() is a classmethod.

image

mhostetter commented 2 years ago

@2bndy5 regarding class properties, in Python 3.9+ they are created like this.

@classmethod
@property
def foo(self) -> str:
    return "bar"

Are you saying that Sphinx still sees foo as a py:property so there would be no way of distinguishing it from a normal instance property?

Other useful links:

2bndy5 commented 2 years ago

Thanks for clarifying how class properties are declared. Personally, I only use instance properties and haven't had a need to use class methods.

As for the "CM" not showing, I'm not sure what to advise there. Obviously, I didn't test my suggestion.

2bndy5 commented 2 years ago

Given the evaluation from https://github.com/jbms/sphinx-immaterial/pull/99#issuecomment-1170535656, this might be better solved upstream in sphinx itself.

jbms commented 2 years ago

Agreed --- I filed https://github.com/sphinx-doc/sphinx/issues/10617