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

itunespy can't set attribute #84

Closed
danimateo opened this issue Jul 10, 2020 · 15 comments
Closed

itunespy can't set attribute #84

danimateo opened this issue Jul 10, 2020 · 15 comments
Labels

Comments

@danimateo
Copy link
Contributor

def get_from_itunes(SONG_NAME):
    """Try to download the metadata using itunespy."""
    # Try to get the song data from itunes
    try:
        SONG_INFO = itunespy.search_track(SONG_NAME)
        # Before returning convert all the track_time values to minutes.
        for song in SONG_INFO:
            song.track_time = round(song.track_time / 60000, 2)
        return SONG_INFO
    except Exception:
        pass

The code above tries to set the track_time, but that setter isn't defined in the itunespy library, so it doesn't work and doesn't even throw an error. This is related to #77 as it only get songs from gaana. The solution would be to remove setting track_time, since something else is already implemented in the source library.

I made a pull request for this, also adding a print for exceptions instead of just pass.

@danimateo danimateo mentioned this issue Jul 10, 2020
@deepjyoti30
Copy link
Owner

@danimateo I just checked the code by exec it on console, it's working all right, why do you think it is failing there?

>>> import itunespy
>>> s = itunespy.search_track('Psycho')
>>> s[0].track_time
221446
>>> s[0].track_time = round(s[0].track_time / 60000, 2)
>>> s[0].track_time
3.69

@danimateo
Copy link
Contributor Author

Is it possible that you have an older version of itunespy? I checked the source code from their repo and there's no setter for track_time.

@deepjyoti30
Copy link
Owner

@danimateo I have the latest version.

It is possible to update the vars without the setters. Since I am accessing those same variables in other places as well. The issue is probably something else.

@danimateo
Copy link
Contributor Author

Maybe it's something else, but I get the "can't set attribute" error. Weird, I will investigate some more.

@danimateo
Copy link
Contributor Author

So, here is the previous commit to the ResultItem class in itunespy and here is the newest one. The old one uses normal python attributes and the new one uses @property decorators, which are read only unless a setter is defined.

class ResultItem(object):
    @property
    def track_time(self):
        return "test"

test_result = ResultItem()
test_result.track_time = "test_new"

The abovde code throws:

Traceback (most recent call last):
  File "testpy.py", line 8, in <module>
    test_result.track_time = "test"
AttributeError: can't set attribute

Could you please check again? Anyway, cool script :) I was looking for something like this for some time.

@deepjyoti30
Copy link
Owner

@danimateo Is this code deployed in the latest version?

I have the latest version that is available in PyPi.

@danimateo
Copy link
Contributor Author

Yes, I have installed it with pip as well. 1.6.0 is the latest version and that's the one I have. Here it is on PyPi and here is the tag on their repo for this version. Can you do a pip freeze and check your version?

@danimateo
Copy link
Contributor Author

Can you try doing pip install --no-cache-dir --upgrade itunespy maybe? Sometimes pip is weird and prefers old local packages instead of remote ones.

@deepjyoti30
Copy link
Owner

@danimateo I already did a pip freeze when I told you I'm using the latest version.

@danimateo
Copy link
Contributor Author

Ok then, I tested this in docker so everything is clean and the error persists. You can try:

docker run -i -t tensorflow/tensorflow:latest-py3 /bin/sh

apt update
apt install ffmpeg

pip install ytmdl

and then you can try doing what you suggested here

Or if you downloading tensorflow just for this is too much, I think the point can be proven by just doing

docker run -i -t python:3.7-alpine /bin/sh
pip install itunespy

and then go at it from the python shell.

@deepjyoti30
Copy link
Owner

deepjyoti30 commented Jul 11, 2020

Hey @danimateo I just tried the code I shared with you earlier and I am getting the can't set attr error now. It's weird, I have not done anything different, no upgrades nothing, I had just turned on the computer and tried running the same code and now I'm getting the error.

Thanks for pointing it out.

Also, since a setter is not defined in the latest release and it is important for us to update that track_time value, I guess we are going to have to revert back to the last supported version and make it the default required version for now.

In the meantime, we can open an issue in itunepy repo requesting the owner to add support for the setter.

Update: Just checked with the last version of itunespy and the code is working all right, I think I'll force the 1.5.5 version

@danimateo
Copy link
Contributor Author

Yeah I've seen that everything works fine without setting the track_time attribute, but there may be some edge cases I haven't considered. For now, downgrading to 1.5.5 seems like a good option. But I will make a PR for adding unidecode and some exception checking, I think those are good to have right now.

@deepjyoti30
Copy link
Owner

@danimateo Sure, I'll raise a Feature request in the official repo to ask for a setter method in the next release.

Also, when you want to throw exceptions, please do a check. Instead of using print, use the logger. That way if the exception is not major, you can just call the warning method and if it is critical you can call the critical method.

Please try to avoid using print's etc.

@danimateo
Copy link
Contributor Author

Yes, my bad. I'll use the logger.

@stale
Copy link

stale bot commented Oct 24, 2020

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs for another 7 days. Thank you for your contributions.

@stale stale bot added the stale label Oct 24, 2020
@stale stale bot closed this as completed Oct 31, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants