-
Notifications
You must be signed in to change notification settings - Fork 52
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
[python-pytrakt] Refactor: Add IdsMixin to replace extract_ids() utility method #186
base: master
Are you sure you want to change the base?
Conversation
It's actually more confusing than that, seems the _id suffixed ones are abandoned:
as the ids property returns without the "_id":
|
46db657
to
9a199c6
Compare
Also, the existing extraction is inconsistent. not applied for TVShow and Movie in this example:
|
2643789
to
2157af6
Compare
Can't finish UserList as it uses namedtuple:
|
A change is needed to namedtuple definitions (UserList):
since namedtuple doesn't allow optional values, this would be a breaking change, meaning this needs to land in 4.x |
1cbaa2d
to
1a5a46b
Compare
d565a62
to
008c34c
Compare
This is ready now, but not sure if this breakage is big enough that this needs to go to 4.x? if you're not creating |
008c34c
to
f8f7a2e
Compare
External projects calling |
Extracting the "ids" on each response is tedious, and such duplication results users being confused:
Also, the existing extraction is inconsistent (Not applied for TVShow and Movie in this example):
PyTrakt/trakt/sync.py
Lines 331 to 342 in 8a6d4f1
This will make the ids extraction automatic, and prevent updating the attributes directly.
Additionally it came out that properties like
tmdb_id
,imdb_id
,trakt_id
are unused. they are initialized asNone
in the constructor but never any other value. these are commented with@deprecated
and are to be removed in 4.x.TODO: