add missing trakt fields from trakt_list#1573
add missing trakt fields from trakt_list#1573gazpachoking merged 5 commits intoFlexget:developfrom Andy2244:trakt_fix
Conversation
gazpachoking
left a comment
There was a problem hiding this comment.
Other than the series_ ones, looks good to me.
| trakt_id = item['show']['ids']['trakt'] | ||
| listed_series[trakt_id] = { | ||
| 'series_name': '%s (%s)' % (item['show']['title'], item['show']['year']), | ||
| 'series_year': item['show']['year'], |
There was a problem hiding this comment.
I'm not sure we should set this. The series_ fields are meant to come from the series plugin with no lookups required.
There was a problem hiding this comment.
I mean movie_year is also handled this way, so why is series_ a exception? Why not fill in all valid information we can get at a particular input stage?
As example i don't use the series plugin and feed the trakt list directly to my search plugin.
There was a problem hiding this comment.
Well, we have more of a built in concept of year for the movie stuff, and the offline parser fills it. With series, due to historical reasons, we have not had an explicit year involved, it's always just been lumped in in the series title.
There was a problem hiding this comment.
My argument would be that it already fills in the series_name using the year from trakt. so why should it not also fill in the series_year, so those two fields are consistent.
I can remove the fill in, just seems strange to me that it uses the year in the name and trakt_year fields, yet omits it for the general field?
There was a problem hiding this comment.
why should it not also fill in the series_year, so those two fields are consistent.
Because it seems to imply that the series plugin has some sort of support for series_year, when it doesn't. I think it would be confusing to see that field populated for your series when you enable the trakt_lookup plugin, but not have it be there otherwise. When you enable the trakt_lookup plugin, you can explicitly access the year via trakt_series_year.
EDIT: The movie_ namespace is a bit different, because there isn't a movie plugin, those fields are just a gentleman's agreement that all the plugins dealing with movies look at. And they all know about the movie_year. No plugins dealing with series know about series_year, so I don't think we should add it anywhere unless we intend to make it consistent across every series plugin, including the actual series plugin.
There was a problem hiding this comment.
You are the expert and i don't really care, since i don't use the field either.
However here is something i find strange. Why is filling a data field dependent on use knowledge and not available information and trusted source? I mean i trust the trakt year, so copying it to the generic field seems valid to me. In comparison i don't trust the newznab tagged "tvairdate", so i did not copy this date to the generic field and left it in the newznab namespace. Yet i trust the "size" and copy this to 'content_size'. In the same sense we trust the "tvdb_id" comming from trakt, so we copy it, instead of leaving it in the trakt namespace.
So to me if i trust the source, i have no problem populating the generic fields, if the information is available. Just my personal thoughts, so just tell me if i should remove it or not so we can get this merged.
There was a problem hiding this comment.
It's got nothing to do with 'trusted source'. The field series_year is not used anywhere, so there's no point starting now. series plugin needs a bigger overhaul and that is beyond the scope of this PR.
I can see that it is set here
Flexget/flexget/plugins/list/imdb_list.py
Line 234 in 1b2b13c
There was a problem hiding this comment.
The user could use this field via set/if/sort plugins, custom.render(). Its just convenient to have general series_year field, especially since omitting or not omiting the year from title depends on the task flow.
So just to give a example, you want to set the title for a task to 'series_name' + year, assuming you normally omit this from you inputs. File rename task or custom search tasks come to mind. Now instead of using 'series_year' the same way you could use 'movie_year', you need specific information what metadata_lookup was used and successfully did the lookup and than separately check the namespace fields like imdb_year, trakt_series_year, tvmaze_series_year. My argument is why do i care which meta lookup populated this field as long as its valid. If the user has multiple meta lookup defined, why should he care which of those had a successful lookup and populated the year field, so it can be used?
There was a problem hiding this comment.
I get your intentions, but as we've said, it's for historical reasons. Doesn't mean you don't have a valid point, it's just not something that belongs in this PR.
|
Looks like tests need a bit of updating as well. |
|
Are there still problems with this PR? |
|
Well, you haven't fixed the tests https://travis-ci.org/Flexget/Flexget/jobs/185735702 |
|
Thanks! |
Motivation for changes:
Some trakt fields where not set for trakt_list imports
Detailed changes:
added:
trakt_movie_name, trakt_movie_year, series_year, trakt_series_name, trakt_series_yearI also noticed that there are multiple ways of handling the year and it looks somewhat inconsistent between plugins.
yearseries_yearmovie_year