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

Add edition to patterns #61

Closed
wants to merge 2 commits into from
Closed

Conversation

dreulavelle
Copy link

@dreulavelle dreulavelle commented Jan 14, 2024

This could possibly use some slight tweaking, but it works well. Let me know if anything needs changed!

Added 2 tests as well.

Ex:

"The.Lord.of.the.Rings.The.Fellowship.of.the.Ring.2001.EXTENDED.2160p.UHD.BluRay.x265.10bit.HDR.TrueHD.7.1.Atmos-BOREDOR"

{
  "resolution": "2160p",
  "quality": "Blu-ray",
  "year": 2001,
  "codec": "H.265",
  "audio": "Dolby TrueHD 7.1",
  "extended": true,
  "bitDepth": 10,
  "hdr": true,
  "edition": "Extended Edition",
  "title": "The Lord of the Rings The Fellowship of the Ring",
  "encoder": "BOREDOR"
}

@mhdzumair
Copy link
Contributor

mhdzumair commented Jan 25, 2024

@dreulavelle update the PR target branch as dev

@dreulavelle dreulavelle changed the base branch from master to dev January 25, 2024 01:31
@dreulavelle
Copy link
Author

Done, my apologies!

@dreulavelle
Copy link
Author

Feel free to edit the regex as you like.

(r"\bUltimate{d}Edition\b".format(d=delimiters), "Ultimate Edition"),
(r"\bExtended{d}Director\"?s\b".format(d=delimiters), "Directors Cut"), # Needs to be before "Extended"
(r"\bExtended\b", "Extended Edition"),
(r"\bDirector\"?s{d}Cut\b".format(d=delimiters), "Directors Cut"),
Copy link
Contributor

Choose a reason for hiding this comment

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

you may combine the "Directors Cut"

Copy link
Author

Choose a reason for hiding this comment

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

I can fix this tonight, thanks! Did you just want it on one line separated with a pipe? Or just cover director instead of both edge cases?

Copy link
Author

Choose a reason for hiding this comment

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

Ah I'll combine, checking extended first

Copy link
Contributor

Choose a reason for hiding this comment

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

one line separated with a pipe

@platelminto
Copy link
Owner

platelminto commented Jan 26, 2024

Hey! Thanks for opening the PR.

Extended editions & directors cuts are already fields: extended and directorsCut. If the current patterns aren't matching something you'd like to match, you can add to those, and add the relevant titles as tests. I'm unsure if an extended director's cut would currently match for both fields, and if you can provide an example title for this, we can go from there.

As for "Ultimate" and "Anniversary" editions, we can add these if there are enough example titles to work from - I don't like adding new fields unless I see several examples of them from real torrents. If you can provide these (just the titles though! don't include torrent URLs here), we can think about adding them. This would likely be as 2 separate fields, similar to extended and directorsCut.

Also, please make sure all tests pass when opening a PR! If you had run them, you might have noticed the duplicate fields. Maybe re-read Contributing. Additionally, the second test you provided doesn't make sense - the output given doesn't match the input. Based on the input though, I've improved the extended pattern, and added an adapted version of the title to the tests. dev should already contain these changes. Unfortunately, the full collection title will still struggle to process correctly due to #20.

Thanks again, and let me know your thoughts!

@dreulavelle dreulavelle deleted the dev branch September 24, 2024 05:05
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