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

Null Attributes should not be excluded from the Release Object #129

Open
rexovas opened this issue May 5, 2023 · 5 comments
Open

Null Attributes should not be excluded from the Release Object #129

rexovas opened this issue May 5, 2023 · 5 comments

Comments

@rexovas
Copy link

rexovas commented May 5, 2023

I've come across a few instances where, say - the "country" attribute is missing from a release

Take this one for example:
https://www.discogs.com/release/18424870-The-Smashing-Pumpkins-Rotten-Apples-Greatest-Hits

In the case of the missing year, I'm still able to access the year attribute without a KeyError

>>> release.data["year"]
0

However when attempting to access the country attribute:

release.data["country"]
Traceback (most recent call last):
  File "/Applications/PyCharm.app/Contents/plugins/python/helpers/pydev/_pydevd_bundle/pydevd_exec2.py", line 3, in Exec
    exec(exp, global_vars, local_vars)
  File "<input>", line 1, in <module>
KeyError: 'country'

This makes it rather clunky to handle cases where certain attributes are missing - because in some cases after catching a KeyError, I simply need to call release.refresh() and the keys may then become accessible (side note, would be nice if I didn't have to call release.refresh())

But if the key is truly missing, a value (such as 0 in the case of the year) can convey that.

I'm not sure if this is an api client issue or an issue from Discogs side

@AnssiAhola
Copy link
Contributor

Hey @rexovas
Thanks for reporting this.

I tried printing out the raw response from the discogs API and it indeed seems like the year is 0 and the country is missing

print(client._get("https://api.discogs.com/releases/18424870"))

This makes it rather clunky to handle cases where certain attributes are missing - because in some cases after catching a KeyError, I simply need to call release.refresh() and the keys may then become accessible (side note, would be nice if I didn't have to call release.refresh())

You can use the fetch method to get the attributes instead of using the data attribute directly.
This refreshes the object once if the given attribute wasn't found initially and tries again.

release.fetch("year")
release.fetch("country")

Also does it not work if you access the release properties normally?

release.year
release.country

This uses the fetch method in the background so it should refresh automatically.

@rexovas
Copy link
Author

rexovas commented May 10, 2023

Thanks - will give these a try and report back

@rexovas
Copy link
Author

rexovas commented May 30, 2023

Pardon the delay - reporting back.

Your suggestion of release.fetch() worked perfectly for my use case. It prevented the need for me to try except KeyError and call release.refresh().

Regarding the above scenario, for year and country

release.fetch('year') returns 0
and release.fetch("country") returns None

One thing to note, I did notice slight differences between using fetch or accessing the release properties normally. For example release.fetch("labels") yields something like

>>> release = client.release(17)
>>> labels = release.fetch('labels')
>>> labels
[{'name': 'Intrinsic Design', 'catno': 'ID001-6', 'entity_type': '1', 'entity_type_name': 'Label', 'id': 26, 'resource_url': 'https://api.discogs.com/labels/26', 'thumbnail_url': 'https://i.discogs.com/Pvo5Ml9Ub4ITB7mFVKW--AZ5L169FOIcFX1du89FeXo/rs:fit/g:sm/q:40/h:228/w:600/czM6Ly9kaXNjb2dz/LWRhdGFiYXNlLWlt/YWdlcy9MLTI2LTE2/NDI2NTg3OTUtNDc1/Ni5qcGVn.jpeg'}]

>>> labels[0]["name"]
'Intrinsic Design'

Whereas release.labels results in:

>>> release = client.release(17)
>>> release.labels
[<Label 26 'Intrinsic Design'>]

>>> release.labels[0]["name"]
TypeError: 'Label' object is not subscriptable

Just wanted to note that the results are not always the same.

Edit:

It does seem to work with this other syntax

>>> release.labels[0].name
'Intrinsic Design'

@JOJ0
Copy link
Contributor

JOJ0 commented Sep 4, 2024

@AnssiAhola are we considering to add things that are actually missing in the API already to the client? If we start to do that, there is no end. Isn't this kind of out of scope? Just thinking out loud if this should be a reasonable feature request or a "works as designed".

Or am I mistaken and we return a default value of "None" already when requesting something that is not existing in the API already?

release.country

I couldn't figure it out from the discussion above...

@AnssiAhola
Copy link
Contributor

@AnssiAhola are we considering to add things that are actually missing in the API already to the client? If we start to do that, there is no end. Isn't this kind of out of scope? Just thinking out loud if this should be a reasonable feature request or a "works as designed".

Or am I mistaken and we return a default value of "None" already when requesting something that is not existing in the API already?

release.country

I couldn't figure it out from the discussion above...

The client uses None as the default value for data that is missing from the response, yes.

I'm fine with "works as designed" for now, we can come back to this if it ever comes up.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Development

No branches or pull requests

3 participants