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

[Lumni] Add new extractor #26532

Closed
wants to merge 3 commits into from
Closed

[Lumni] Add new extractor #26532

wants to merge 3 commits into from

Conversation

Surkal
Copy link
Contributor

@Surkal Surkal commented Sep 6, 2020

Please follow the guide below

  • You will be asked some questions, please read them carefully and answer honestly
  • Put an x into all the boxes [ ] relevant to your pull request (like that [x])
  • Use Preview tab to see how your pull request will actually look like

Before submitting a pull request make sure you have:

In order to be accepted and merged into youtube-dl each piece of code must be in public domain or released under Unlicense. Check one of the following options:

  • I am the original author of this code and I am willing to release it under Unlicense
  • I am not the original author of this code but it is in public domain or released under Unlicense (provide reliable evidence)

What is the purpose of your pull request?

  • Bug fix
  • Improvement
  • New extractor
  • New feature

Description of your pull request and other information

French website for school children.
Requested in #24574 and #25772.

@dstftw dstftw force-pushed the master branch 2 times, most recently from 5e26784 to da2069f Compare September 13, 2020 13:49
@llevrel
Copy link

llevrel commented Oct 2, 2020

Hi,

Why has the "Lumni" commit not be applied? I'm eagerly waiting for it XD

Thanks

Copy link
Contributor

@dirkf dirkf left a comment

Choose a reason for hiding this comment

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

Happy to re-review and merge once FranceTV IE is working again.

@@ -580,6 +580,7 @@
from .localnews8 import LocalNews8IE
from .lovehomeporn import LoveHomePornIE
from .lrt import LRTIE
from .lumni import LumniIE, LumniPlaylistIE
Copy link
Contributor

Choose a reason for hiding this comment

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

More maintainable:

Suggested change
from .lumni import LumniIE, LumniPlaylistIE
from .lumni import (
LumniIE,
LumniPlaylistIE,
)

Comment on lines +67 to +75
entries = [self.url_result(
'https://lumni.fr/video/%s' % video_id,
ie=LumniIE.ie_key(), video_id=video_id)
for video_id in orderedSet(re.findall(
r'<a[^>]+\bhref=["\']/video/([0-9a-z-]+)', webpage))]

playlist_title = self._og_search_title(webpage)

return self.playlist_result(entries, playlist_id, playlist_title)
Copy link
Contributor

Choose a reason for hiding this comment

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

There's a utility method for this; also the regex can be relaxed:

Suggested change
entries = [self.url_result(
'https://lumni.fr/video/%s' % video_id,
ie=LumniIE.ie_key(), video_id=video_id)
for video_id in orderedSet(re.findall(
r'<a[^>]+\bhref=["\']/video/([0-9a-z-]+)', webpage))]
playlist_title = self._og_search_title(webpage)
return self.playlist_result(entries, playlist_id, playlist_title)
playlist_title = self._og_search_title(webpage)
return self.playlist_from_matches(
re.findall(r'''<a\b[^>]+\bhref\s*=\s*["']/video/([0-9a-z-]+)''', webpage),
playlist_id, playlist_title,
getter=lambda x: 'https://lumni.fr/video/' + x,
ie=LumniIE.ie_key())

Comment on lines +50 to +52
'title': 'Les Fondamentaux : Vocabulaire',
},
'playlist_mincount': 39
Copy link
Contributor

Choose a reason for hiding this comment

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

Currently these are right:

Suggested change
'title': 'Les Fondamentaux : Vocabulaire',
},
'playlist_mincount': 39
'title': 're:(?i)^Les Fondamentaux : Vocabulaire$',
},
'playlist_mincount': 25


from .common import InfoExtractor
from .francetv import FranceTVIE
from ..utils import orderedSet
Copy link
Contributor

Choose a reason for hiding this comment

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

Removed by final suggestion:

Suggested change
from ..utils import orderedSet

Comment on lines +32 to +34
full_id = 'francetv:%s' % video_id

return self.url_result(full_id,
Copy link
Contributor

Choose a reason for hiding this comment

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

Roll this together:

Suggested change
full_id = 'francetv:%s' % video_id
return self.url_result(full_id,
return self.url_result( 'francetv:' + video_id,

@dirkf dirkf closed this Aug 1, 2023
@dirkf dirkf added the defunct PR source branch is not accessible label Oct 2, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
defunct PR source branch is not accessible
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants