-
Notifications
You must be signed in to change notification settings - Fork 92
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
Upgrade from guessit 3.1.1 to 3.3.1 breaks custom rebulk rules #692
Comments
Temporarily fix guessit until guessit-io/guessit#692 is fixed.
Your issue is with no doubt caused by those mutations. I'll try to find another solution though, as I don't want pattern and regex keywords to be passed to underlying function. |
Let me know when you have a solution. I'm happy to test it. 😉 |
Temporarily fix guessit until guessit-io/guessit#692 is fixed.
Could you give a try with |
@Toilal |
Guessit with custom rebulk rules are broken.
I'm using guessit as mentioned below:
Afterwards I'm using the adapted default_api:
The
default_api.guessit
calls internally again theconfigure()
Within the
configure()
it compares if theadvanced_config
has changed and forces a rebuild of rebulk when different.Commit 51e0021#diff-66e80a71bd17399af0e8d250f0b561e30ca0090d84cd4b306ea13bfdfc90f842 has introduced a breaking change when guessit is extended with custom rebulk rules.
Within this change, the
pattern
andregex
are popped from the config options (really removed from the advanced_config) which breaks the comparison in theconfigure()
in theGuessitApi
class inapi.py
.Because the
advanced_config
is no longer identical, rebulk is rebuild and my custom rebulk rules are lost...On 3.1.1 this was not the case...
Fix seems fairly easy and is tested by me:
pattern.pop('pattern')
bypattern.get('pattern')
kwargs.pop('regex', False)
bykwargs.get('regex', False)
This doesn't physically removes the items from the dict and ensures that the config is not changed, so the rebulk rules are not lost.
Can you please verify if my findings are correct and fix it in case you agree?
Thanks!
The text was updated successfully, but these errors were encountered: