Add check for missing year in models Release and Master#144
Add check for missing year in models Release and Master#144akda5id wants to merge 1 commit intojoalla:masterfrom
Conversation
and Master, if so, make it None
| super(Release, self).__init__(client, dict_) | ||
| self.data['resource_url'] = '{0}/releases/{1}'.format(client._base_url, dict_['id']) | ||
| if not 'year' in dict_: | ||
| dict_['year'] = None |
There was a problem hiding this comment.
Hi, sorry for the delay in responding, I'm travelling and try to stay offline as much as I can ;-) Also excuse any sloppy formatting, I try to do my best while editing on mobile...
Anyway, interesting, and yes it's a quickfix but I'm wondering how we could make it a fix for potentially all fields that could be missing.
I'm probably just thinking out loud and havent thought all this through thoroughly enough, but:
We are defining year as a SimpleField here:
discogs_client/discogs_client/models.py
Line 501 in 6153797
Why wouldnt it be possible to fall back to None here already?
discogs_client/discogs_client/models.py
Line 152 in 6153797
descriptor class:
discogs_client/discogs_client/models.py
Line 23 in 6153797
the ObjectDescriptor class does have some kind of fallback to None, we could try to do something similar in the SimpleDescriptor:
discogs_client/discogs_client/models.py
Line 66 in 6153797
There was a problem hiding this comment.
Doesnt year default to 0 if no year is specified so there should always be a value
3 participants
If so, make it None.
See for discussion #143