subliminal Support BSPlayer Subtitles provider

Apr 05 '20 19:04 Nyaran

Thank you for the contribution. I'll review this whenever I can

May 03 '20 11:05 ratoaq2

Is there any way how to help with merging this? The code seams clean enough, but I'm not familiar with the code enough.

Dec 21 '20 22:12 kepi

bump

Apr 22 '23 21:04 luzpaz

As subliminal seems alive again, I'm working again on this to sync with the latest changes

Jul 28 '24 17:07 Nyaran

I think is ready to review :smiley:

Jul 28 '24 22:07 Nyaran

Thanks! You can find some guidance about the tools we use for linting and typing (ruff/pre-commit and mypy) in the CONTRIBUTING file

The errors with the tests for python < 3.12 seem to be a missing line at the top of the files: from __future__ import annotations

Jul 28 '24 23:07 getzze

It's seems that there is something wrong with Python 3.9 and below, the response from BSPlayer is recorded as "binary", but from Python 3.10, is correctly recorded as a string (check tests/cassettes/bsplayer/test_login.yaml and tests/cassettes/bsplayer.py3.9/test_login.yaml)

The solution I found is to have two different recordings, one for Python <=3.9 (named bsplayer.py3.9) and another for python >=3.10 (named bsplayer)

Do you have any thoughts about this?

Jul 30 '24 23:07 Nyaran

You can try adding the decode_compressed_response=True argument when initializing VCR in test_bsplayer.py, see the other providers.

Jul 31 '24 07:07 getzze

You can try adding the decode_compressed_response=True argument when initializing VCR in test_bsplayer.py, see the other providers.

That is! Thank you! I didn't see that.

I also fixed the mypy warnigns, so now is really ready for review

Pd. The mypy analysis is not documented on the contribution guide

Jul 31 '24 18:07 Nyaran

Coverage report

Click to see where and how coverage changed

File	Statements	Missing	Coverage	Coverage (new stmts)	Lines missing
subliminal/providers
bsplayer.py					107-111, 116-120, 125-129, 171-175, 177, 193, 218
subliminal/refiners
hash.py
Project Total

_{This report was generated by python-coverage-comment-action}

Aug 01 '24 00:08 github-actions[bot]

Hey! Anything else I need to do before this PR can be reviewed/merged?

Aug 15 '24 14:08 Nyaran

Hi @Nyaran, Thanks a lot for your contribution!

I made some modifications about types and making sure connection errors are correctly handled.

I have one question though, you initialize a self.server attribute to a ServerProxy but it is never used, was it a remnant of older code?

Sep 07 '24 23:09 getzze

(failing CI for docs does not seem to be related to this PR)

Sep 08 '24 00:09 getzze

I have one question though, you initialize a self.server attribute to a ServerProxy but it is never used, was it a remnant of older code?

Looks like remnant code, should be safe to remove it

Sep 08 '24 00:09 Nyaran

Ok, LGTM! @ptrcnull do you have time to have a quick look?

Sep 08 '24 00:09 getzze

Merged!

Thanks again @Nyaran for the PR and updating to the latest change!!

Sep 16 '24 20:09 getzze

Congrats! Long time coming

Sep 16 '24 22:09 luzpaz