-
Notifications
You must be signed in to change notification settings - Fork 4
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
Implement a custom __repr__
#328
Comments
Ideas:
Notes:
class Context:
session: requests.Session
url: Url
_version: str
def __init__(self, session: requests.Session, url: Url):
self.session = session
self.url = url
# self._version is retrieved on demand of `.version` property
@property
def version(self) -> Optional[str]:
try:
return self._version
except AttributeError:
endpoint = self.url + "server_settings"
response = self.session.get(endpoint)
result = response.json()
value = self._version = result.get("version")
return value
@version.setter
def version(self, value):
self._version = value |
My understanding is that implementing side note; using class variables this way is incorrect.
This implies that these variables are shared across all instances of the class. Since we support creating multiple |
Thinking through this a bit more. I'm not convinced implementing repr in this way is worth the effort. My opinion is that the current implementations using |
Thanks for all the context and details here. Very much agree that if there's doubt about if we'll need it, we should continue to build out the SDK so that we can gather more experience + information on if it's needed.
I might not be following but in the last sentence here did you mean |
Yes, I meant "list of dictionaries". Sorry about that. |
Please use the default methods from
dict
for the time being. In the future, we can look into implementing true__repr__
which would provide true serliaization functionality if it is necessary. See https://docs.python.org/3/library/functions.html#reprOriginally posted by @tdstein in #300 (comment)
The text was updated successfully, but these errors were encountered: