Skip to content
This repository has been archived by the owner on Oct 18, 2024. It is now read-only.

Add Hungarian provider Hosszupuska #445

Merged
merged 3 commits into from
Mar 6, 2018
Merged

Add Hungarian provider Hosszupuska #445

merged 3 commits into from
Mar 6, 2018

Conversation

morpheus133
Copy link

@pannal
Copy link
Owner

pannal commented Feb 28, 2018

Thanks. A couple of questions:

  • Why the require(lxml)? The PMS infrastructure already supplies lxml.
  • You should consider using the mixin subliminal_patch.providers.mixins.ProviderSubtitleArchiveMixin and use self.get_subtitle_from_archive() to extract the requested subtitle from the zip archive.

@morpheus133
Copy link
Author

Answer to both:
I also create this provider to Subliminal and try to have as minimal different as possible:
Diaoul/subliminal#809

  • require(lxml)
    For subliminal I need to support also html_parser not just lxml, and this was left here. (I can remove becuse here it is not neceserry)
    -mixin
    I will check this and add it.

- Remove LXML checking  (Needed only for official subliminal)
- Added fix_inconsistent_naming handling
@morpheus133
Copy link
Author

I checked all your comments and implemented them.

@pannal
Copy link
Owner

pannal commented Mar 1, 2018

Great, for that to work your Subtitle subclass needs to implement the attributes releases and asked_for_episode, though.

@morpheus133
Copy link
Author

What is the difference between subtitle.asked_for_episode and subtitle. episode?
At the other providers it seems the same. Or I missed something?

@pannal
Copy link
Owner

pannal commented Mar 2, 2018

Subtitle.episode should be the episode returned by the provider, not the one searched for, to circumvent bad matches. asked_for_episode should be the one that was searched for.

…ible in other providers.

- Add asked_for_episode
@morpheus133
Copy link
Author

I made all the modification, you mentioned.

@pannal
Copy link
Owner

pannal commented Mar 6, 2018

Great, thank you!

@pannal pannal merged commit 5598ee0 into pannal:master Mar 6, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants