Considering accessors versus properties on models. about hikari HOT 13 CLOSED

Do you have any idea how this would work with builder cache objects? And would we maybe have a method that returns a dict of the raw values or something like that and is on all models?

from hikari.

Do you have any idea how this would work with builder cache objects? And would we maybe have a method that returns a dict of the raw values or something like that and is on all models?

You'd probably have to look at private fields and remove the underscore directly for the constructor to work around this.

from hikari.

Do you have any idea how this would work with builder cache objects? And would we maybe have a method that returns a dict of the raw values or something like that and is on all models?

You'd probably have to look at private fields and remove the underscore directly for the constructor to work around this.

If you're fine with looking at private fields then it shouldn't be too hard to convert the data class over then

from hikari.

Do you have any idea how this would work with builder cache objects? And would we maybe have a method that returns a dict of the raw values or something like that and is on all models?

You'd probably have to look at private fields and remove the underscore directly for the constructor to work around this.

If you're fine with looking at private fields then it shouldn't be too hard to convert the data class over then

It is internal to the library. As long as we do not encourage users to do this, we shouldn't have a problem.

Fields are exposed on the assumption we are responsible developers, tests should highlight bad stuff :^)

from hikari.

I feel like the cons outweight the pros quite a bit in this case. Python is a syntatically simple language in general, and the ease of use is one of its key features, and certainly one that attracts a lot of people. Making the library super verbose by making everything a call to a get_foo() method will substantially reduce the simplicity and pain-less aspect of this library, which I'm afraid will make people gravitate towards solutions like discord.py more in the future. Even though Hikari is, in my opinion, miles better than discord.py due to its fully typed, documented and tested nature, I'm under the impression that it would level out, or even make Hikari less desireable, as its nice pros would be muffled by the high verbosity and "ugliness" of code using it.
I see how this is a quite complicated decision however. Having these calls would simplify stuff and make everything more consistent... You say it's not an anti-pattern per-se and that many modules use it, but I in all honesty, I fail to recall any. Could you point me some?

Thanks 🅱️iscord

from hikari.

You say it's not an anti-pattern per-se and that many modules use it, but I in all honesty, I fail to recall any. Could you point me some?

Pretty much all standard lib modules like asyncio, thread, logging, do this.

Mainly they use future.result() instead of future.get_result() but the concept is the same.

If people are happy that you have to be mindful of how you use event entities, for example, then I am fine with keeping stuff how it is.

An alternative to this may be using custom overrides of __dir__ to exclude properties on instances.

This fixes the introspection issue by allowing inspect.getmembers to skip stuff we deem slow. We could follow what Python's enum type does, which is only specify specific values (in this case slots only, or attrs fields only).

>>>
>>> class Foo:
...     def bar(self): pass
...     def baz(self): pass
...     def bork(self): pass
...     def __dir__(self): return ["bar", "baz"]
...
>>> import inspect
>>>
>>> inspect.getmembers(Foo())
[('bar', <bound method Foo.bar of <__main__.Foo object at 0x7395e7afa0>>), ('baz', <bound method Foo.baz of <__main__.Foo object at 0x7395e7afa0>>)]
# notice no bork

from hikari.

Might close this for the time being and reopen it if I still feel it is relevant.

from hikari.

@FasterSpeeding @davfsa please.

If we are going ahead with this, we need to look into the pros and cons of doing this and ensure we have it noted down before we make a bad decision.

from hikari.

As I said through Discord on the day this was opened (should have done it through here) I dont really mind one or another. Its true that properties might be a more "pythonish" approach.

I think its fine to have get_x in a lot of cases (eg, get_channel or get_guild). But for simple or inmutable arguments that wont change (eg id) I think its fine to have them as properties.

from hikari.

I think its fine to have get_x in a lot of cases (eg, get_channel or get_guild). But for simple or inmutable arguments that wont change (eg id) I think its fine to have them as properties.

How would you suggest handling events and inheritance hierarchy in this case?

Consider GuildEvent which has a guild property.

Both GuildAvailable and GuildUnavailable provide this attribute in different ways. The former is always aware of the field directly as it is part of the low-level event payload, whereas the latter has to do a cache call for this (calls the GuildEvent default property).

Should these be properties, getter methods, or just remove this from the base class altogether and provide .guild on an available event and .get_guild() on an unavailable event, in your opinion?

from hikari.

Should these be properties, getter methods, or just remove this from the base class altogether and provide .guild on an available event and .get_guild() on an unavailable event, in your opinion?

In this case I would use get_guild() for both.

I probably did not explain myself correctly before. I meant if its something that we can assure will always be there (eg id) and we dont have to make any calls for it in any variant of the object, then its a property, else, a get_x. That sound better?

Edit: this might cause a lot of inconsistencies. Tbh, i would first do #49 and try to get them as simple as we can to later implement this

from hikari.

@davfsa suggestions on what that involves?

from hikari.

For now I think we have avoided the issue, so will close.

Will reopen if/when this is needed.

from hikari.