sphinx-immaterial icon indicating copy to clipboard operation
sphinx-immaterial copied to clipboard

Distinguish between class and instance methods/properties

Open mhostetter opened this issue 3 years ago • 8 comments

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.

mhostetter avatar May 16 '22 20:05 mhostetter

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 avatar May 17 '22 00:05 2bndy5

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 avatar May 17 '22 00:05 2bndy5

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.

2bndy5 avatar May 17 '22 00:05 2bndy5

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 avatar May 17 '22 01:05 mhostetter

@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:

  • https://github.com/sphinx-doc/sphinx/pull/9461
  • https://github.com/sphinx-doc/sphinx/issues/9445
  • https://docs.python.org/3.9/library/functions.html#classmethod

mhostetter avatar May 17 '22 01:05 mhostetter

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 avatar May 17 '22 03:05 2bndy5

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

2bndy5 avatar Jun 29 '22 22:06 2bndy5

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

jbms avatar Jun 29 '22 22:06 jbms