Confusing when property methods behave like functions and __call__ is undocumented

[samedii]

Edit: For clarity the original title of issue was "Name of methods' arguments in documentation from autodoc".

I was going to create a PR but it seems there is a lot of special sauce in the autodoc configuration. Is there something blocking adding the arguments to the functions in the documentation?

[samedii]

On a very related note: it would also be nice if viewcode worked for the class methods (especially at the moment since the documentation is sparse)

[jjmaldonis]

I don't know what the solution to this is. Do you want to take a stab at it?

[samedii]

I started looking into this and I found out that the reason is that everything is actually a @property so naturally they would not have arguments. However, the code style is like they weren't properties because the underlying object implements __call__. Moreover, the __call__ method does not appear in the documentation at all.

Example:

class Summoner:
    @property
    def match_history(self) -> "MatchHistory":
        from .match import MatchHistory
        return MatchHistory(summoner=self)

class MatchHistory:
    def __call__(self, **kwargs) -> "MatchHistory":
        pass

Recommended usage makes the documentation quite confusing:

for match in cass.Summoner(id=summoner_id).match_history(
    queues={cass.Queue.ranked_solo_fives},
    begin_time=creation.shift(days=-100),
    end_time=creation,
):
    pass

It is like the function call/arguments have been hidden away in the __call__.

It might help a little if the type hints in the documentation linked to the return type's documentation but I didn't easily find an extension that does this.

I will rename the issue since it was based on confusion but is valid in another way.

[iann838]

@samedii I don't see the confusion you have with the __call__, that is the function that gets executed when you call it, and type hints does its work if you do:

summoner = cass.Summoner(id=summoner_id)
match_history = cass.MatchHistory(summoner=summoner, queues={cass.Queue.ranked_solo_fives}, begin_time=creation.shift(days=-100), end_time=creation)
# Above Ghost object will show the proper hints in your editor
for match in match_history:
    pass

And one thing to take in mind, matchlist endpoint takes account_id instead of summoner_id, you should construct the Ghost Summoner using account_id to save you a call to the summoner endpoint.

[samedii]

This is just my opinion but I don't think __call__ should be implemented like this since it will cause confusion about what it's meant to do. I think it would have been better to move the functionality to MatchHistory.filter or something (I see that name is already taken though). There are no clues for the user what the method __call__ will do for a new user but if it had a name like filter then it would make sense.

This is of course made worse by match_history being a property on other classes so it looks like it's a method initially.

Thanks for the tip on initializing Summoner with account_id!

[iann838]

@samedii well, it's Python, you can do it in N ways lol, __call__ to actually call it makes sense to me, and I don't think the filter thing is needed because it is not a filter at cass' objects, it is a filter done on the Riot end, so the filter is implemented on the Riot end and a constructor type is implemented in here, even other API Wrapper don't do filter, they do it with either __init__ or __call__

[samedii]

... even other API Wrapper don't do filter, they do it with either init or call

Ok I didn't know about this convention, if this is the convention then that does indeed inform the user that knows about this.

Feel free to close this if you'd like. I just wanted to put my user experience out there and it's of course completely up to you to decide if you think this pattern is easy or difficult to understand.

Also I think that maybe this would not have been an issue for me if the documentation was less sparse but who knows.

[jjmaldonis]

It would be great if there is a way to specify, and link to, the return types in the documentation. I don't know how to either, but it seems like that should be possible! If you would like to look into it more, and maybe let us know the results of the research you do, that would be very helpful.

There is a tradeoff between lots of functionality and a clean API/interface. I'm sure there are ways to make it better, but it's set in stone now. If you can help us to find ways to better document it that is fantastic!