Skip to content

[change] series begin - SXX accepted as ID / CLI --forget option#1841

Merged
liiight merged 10 commits intodevelopfrom
series_begin-as-season
Jun 1, 2017
Merged

[change] series begin - SXX accepted as ID / CLI --forget option#1841
liiight merged 10 commits intodevelopfrom
series_begin-as-season

Conversation

@tubedogg
Copy link
Copy Markdown
Contributor

Motivation for changes:

Be able to use S02 as a begin identifier, and also forget a begin episode link without removing the history of the episode itself.

Detailed changes:

Two changes relating to begin:

  • Behind the scenes, appends ‘E01’ when an identifier of the type SXX is used as the begin episode. This can be done via CLI series begin SXX, config with series plugin, or API. To make this work, an additional config format validator of episode_or_season_identifier was added which calls parse_episode_identifier with the new parameter identify_season set to True. When this is not wanted, continue to use the existing episode_identifier config format validator.
  • CLI series begin <show> --forget argument to remove begin episode association in series table.

Implemented feature requests:

  • feathub #18

Log and/or tests output (preferably both):

When using an identifier of the format SXX while setting begin via CLI:

`S05` was identified as a season.
Releases for `My Show` will be accepted starting with `S05E01`

When using the --forget argument:

The begin episode for `My Show` has been forgotten.

tubedogg added 2 commits May 25, 2017 01:38
Two changes relating to `begin`:
- Behind the scenes, appends ‘E01’ when an identifier of the type SXX is used as the begin episode. This can be done via CLI `series begin SXX`, config with series plugin, or API. To make this work, an additional config format validator of `episode_or_season_identifier` was added which calls `parse_episode_identifier` with the new parameter `identify_season` set to `True`. When this is not wanted, continue to use the existing `episode_identifier` config format validator.
- CLI `series begin <show> --forget` argument to remove begin episode association in series table.
@tubedogg tubedogg requested a review from liiight May 25, 2017 06:51
identified_by = parse_episode_identifier(ep_id, identify_season=True)
if identified_by == 'ep':
ep_id = ep_id.upper()
if 'E' not in ep_id:
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

This is far too specific to exist in this scope. What about 1x01 episodes? or any other format?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

None of those would have gotten past parse_episode_identifier, which only recognizes SXXEXX (and now SXX).

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

That said I did change it to look for an entity_type of season returned from parse_episode_identifier instead of parsing the string directly, though that function still only recognizes SXXEXX or SXX.

identified_by = 'sequence'
elif re.match(r'(?i)^S\d{1,4}E\d{1,3}$', ep_id):
identified_by = 'ep'
elif re.match(r'(?i)^S\d{1,4}$', ep_id) and identify_season:
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

This is the core point of this whole change. You need the parse function here to return a tuple perhaps, that sets identified by and entity type. Then all other elements that rely on this data should be converted to use that tuple reply.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Addressed in 0497473.

session.flush()
series.begin = episode

def remove_series_begin(series):
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

This is completely redundant, just set to None where needed



@format_checker.checks('episode_or_season_identifier', raises=ValueError)
def is_episode_or_season_identifier(instance):
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

This is a terribly long name

tubedogg added 3 commits May 25, 2017 04:18
It consists of `identified_by` and `entity_type`, the latter of which returns as either `episode` or `season`.
- Other than set_series_begin, the only other places using `parse_episode_identifier` simply checked if the return was `None`, indicating an error, so no changes were necessary.
- Removed remove_series_begin function and moved its functionality into plugins/CLI/series.py.
- Renamed `episode_or_season_identifier` config format validator to `episode_or_season_id`.
except ValueError as e:
console(e)
else:
if re.match(r'(?i)^S\d{1,4}$', ep_id):
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Change this to take type as well

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Okay, this should be addressed in 1240b73.

@tubedogg tubedogg added the CLI label May 29, 2017
@tubedogg
Copy link
Copy Markdown
Contributor Author

tubedogg commented Jun 1, 2017

This should be ready to go.

@liiight liiight merged commit f900a11 into develop Jun 1, 2017
@tubedogg tubedogg deleted the series_begin-as-season branch June 2, 2017 03:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants