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

Properties are rendered with parens #9

Closed
bear24rw opened this issue Mar 22, 2022 · 8 comments
Closed

Properties are rendered with parens #9

bear24rw opened this issue Mar 22, 2022 · 8 comments

Comments

@bear24rw
Copy link

Describe the bug
@property's are rendered with trailing () which makes them look like functions.

To Reproduce

class Example:
    @property
    def foobar(self):
        """
        Return the value of the foobar property
        """
        return 42

Expected behavior
I expect the heading in the API docs to be just foobar but it is rendered as foobar()

System (please complete the following information):

mkdocs==1.2.3
mkdocs-autorefs==0.4.1
mkdocs-material==7.3.4
mkdocs-material-extensions==1.0.3
mkdocstrings==0.18.1
mkdocstrings-python==0.6.6
@pedrolabonia
Copy link

pedrolabonia commented Apr 15, 2022

Since the new griffe version creates labels, should we do it on the templating side? We can do a simple check to see if the Function object passed by griffe has 'property' in object.labels.

I don't know if this can be solved directly on the Jinja template or if we need additional context from Griffe.

If we could have some direction I can try to help.

Im trying to figure out exactly where the connection between griffe and mkdocs-python is.

Edit: I've figured it out, seems easy enough to do it on the template itself. I'm working on it.

@pawamoy
Copy link
Member

pawamoy commented Apr 15, 2022

You guessed right! I was exactly planning to add a check in the templates 🙂 I'm glad to hear you're working on it, should I wait for a PR then 😄 ?

@pedrolabonia
Copy link

pedrolabonia commented Apr 15, 2022

You guessed right! I was exactly planning to add a check in the templates 🙂 I'm glad to hear you're working on it, should I wait for a PR then 😄 ?

Yes!

Also, one question... Right now, @property getters and setters are marked as functions, and you've wrote tests do assert so.

Do you want to keep that design, or should we pass them as attributes from griffe? From what I've saw that's how the old handler dealed with it. Got them as attributes straight from pytkdoc.

Just thinking about "economy" of code here, since griffe is telling all themes it's a function, and we'd need to adjust every theme.

It isn't an issue too, just a matter of which design decision you want to go with.

@pawamoy
Copy link
Member

pawamoy commented Apr 16, 2022

Griffe will always return a property as a function. That's just how it is technically. But when rendering API documentation, it makes sense to categorize them as attributes, since that's how they are used.

EDIT: well I'm wondering if Griffe itself shouldn't categorize properties as attributes directly. Griffe is supposed to build and manipulate API structures so that would make sense as well. I'll have to think about it more.

@pedrolabonia
Copy link

I agree that passing them as attributes from Griffe might be the best way. I can't imagine right now a use case where a property would be bundled with the class methods, but there might be some edge cases.

Doing it in Griffe avoids all the templating patching and logic for that specific case, and feels to me as more of an expected behavior.

If you have any ideas on how to go about this, I can try to help. I'll close the PR for now.

@pawamoy
Copy link
Member

pawamoy commented Apr 16, 2022

Doing it in Griffe avoids all the templating patching and logic for that specific case, and feels to me as more of an expected behavior.

Exactly!

If you have any ideas on how to go about this, I can try to help.

Let me bring the change in Griffe, then I'll give you some guidance to update the Python handler 🙂

@corykinney
Copy link

corykinney commented Oct 11, 2022

Just wondering if there has been any progress on rendering properties as attributes; I'm generating some documentation with quite a few properties and don't want anyone to be confused by them looking like functions.

EDIT: Also, I'm not sure if it's already possible, but it would be great if the attributes would also display the property return type hint.

pawamoy added a commit to mkdocstrings/griffe that referenced this issue Nov 30, 2022
pawamoy added a commit to mkdocstrings/griffe that referenced this issue Nov 30, 2022
@pawamoy
Copy link
Member

pawamoy commented Nov 30, 2022

This is now implemented and will be published with the next release of Griffe.

@pawamoy pawamoy closed this as completed Nov 30, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants