Skip to content

Update npo_watchlist#1664

Merged
liiight merged 3 commits intoFlexget:developfrom
mfonville:develop
Jan 31, 2017
Merged

Update npo_watchlist#1664
liiight merged 3 commits intoFlexget:developfrom
mfonville:develop

Conversation

@mfonville
Copy link
Copy Markdown
Contributor

  • The NPO3 site has been updated to list fragments with the episode too, and list 'articles' also; new trick to only find episodes by looking at running time
  • Add running time as npo_runtime (should also help with the date parsing)
  • Update the test, added npo_runtime check; but also make sure to test for that things that should NOT be listed are asserted as None

Motivation for changes:

NPO.nl site has been changed again

* The NPO3 site has been updated to list fragments with the episode too, and list 'articles' also; new trick to only find episodes by looking at running time
* Add running time as npo_runtime (should also help with the date parsing)
* Update the test, added npo_runtime check; but also make sure to test for that things that should NOT be listed are asserted as None
@@ -201,33 +201,36 @@ def _parse_episodes_npo3(self, task, config, series_name, page, series_info):

entries = list()
for listItem in page.findAll('div', class_='item'):
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I know this wasn't introduced in this PR but please change listItem to list_item

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

alright, will do

e['npo_name'] = series_info['npo_name']
e['npo_description'] = series_info['npo_description']
e['npo_language'] = series_info['npo_language']
if subtitle[1]: # if runtime is available in the subtext
Copy link
Copy Markdown
Member

@liiight liiight Jan 31, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Will the previous split always work? If not, this may raise an IndexError

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The previous split might indeed fail, because there is no easy way to check if 100% of the site confirms to this new subtext format. I will add an extra check

Copy link
Copy Markdown
Contributor Author

@mfonville mfonville Jan 31, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm, I checked the current way, it and this should actually work already correctly... and the split() always returns a list (of e.g. zero length if the splitted symbol is not there);
and the if-statement correctly handles the situation if the key exists in the list. It does not spawn any error with any input.

So I will leave this as it is, if you don't mind.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Python 2.7.11 (v2.7.11:6d1b6a68f775, Dec  5 2015, 20:40:30) [MSC v.1500 64 bit (AMD64)] on win32
Type "help", "copyright", "credits" or "license" for more information.
>>> l = 'nodots'.split('.')
>>> l
['nodots']
>>> l[1]
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
IndexError: list index out of range
>>>

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

but it is not l[1]

it is if l[1]:

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That will still fail. No matter what precedes l[1] it is a direct access to the 2nd element in the list.

>>> if l[1]:
...     pass
...
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
IndexError: list index out of range
>>>

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It'll still throw that error. Do if len(l) > 1

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok, I will update it

@liiight liiight merged commit b90daf5 into Flexget:develop Jan 31, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants