Improve filtering of set types and card layouts

README.rst

@@ -166,7 +166,9 @@ Changelog
 Development
 -----------
 
-- ...
+-   Fix filtering of digital only cards
+-   Switch from inclusion to exclusion for set types
+-   Add exclusion of card layouts (to catch tokens in non-token sets)
 
 2.3.0
 -----

TODO.md

@@ -3,7 +3,7 @@
 -   Group sets by parent set is xlsx output
     -   Block? probably not
     -   At the very least, merge promos into somewhere?
--   Filter card types as well as set types to remove tokens and such in promo sets
+-   Filter non-English card sets
 -   Option to filter out non-English sets / cards
 -   diffs to stdout
 -   Use setuptools-scm so I don't have to think about releases as much

mtg_ssm/containers/bundles.py

@@ -1,8 +1,8 @@
 """Data bundle definitions."""
 
-from typing import List, NamedTuple, Set
+from typing import List, NamedTuple, Optional, Set
 
-from mtg_ssm.scryfall.models import ScryCard, ScrySet, ScrySetType
+from mtg_ssm.scryfall.models import ScryCard, ScryCardLayout, ScrySet, ScrySetType
 
 
 class ScryfallDataSet(NamedTuple):
@@ -12,32 +12,37 @@ class ScryfallDataSet(NamedTuple):
     cards: List[ScryCard]
 
 
-def remove_digital(scryfall_data: ScryfallDataSet) -> ScryfallDataSet:
-    """Filter a ScryfallDataSet to remove all digital only sets and cards."""
-    accepted_sets = []
+def filter_cards_and_sets(
+    scryfall_data: ScryfallDataSet,
+    *,
+    exclude_set_types: Optional[Set[ScrySetType]] = None,
+    exclude_card_layouts: Optional[Set[ScryCardLayout]] = None,
+    exclude_digital: bool = False,
+) -> ScryfallDataSet:
+    """Filter a ScryfallDataSet to exclude desired set types, card layouts, and digital only products."""
     accepted_setcodes = set()
     for set_ in scryfall_data.sets:
-        if set_.digital:
+        if exclude_set_types and set_.set_type in exclude_set_types:
+            continue
+        if exclude_digital and set_.digital:
             continue
-        accepted_sets.append(set_)
         accepted_setcodes.add(set_.code)
 
-    accepted_cards = [
-        c for c in scryfall_data.cards if c.set in accepted_setcodes and not c.digital
-    ]
-    return ScryfallDataSet(sets=accepted_sets, cards=accepted_cards)
-
+    accepted_cards = []
+    nonempty_setcodes = set()
+    for card in scryfall_data.cards:
+        if card.set not in accepted_setcodes:
+            continue
+        if exclude_card_layouts and card.layout in exclude_card_layouts:
+            continue
+        if exclude_digital and card.digital:
+            continue
+        accepted_cards.append(card)
+        nonempty_setcodes.add(card.set)
 
-def filter_set_types(
-    scryfall_data: ScryfallDataSet, set_types: Set[ScrySetType]
-) -> ScryfallDataSet:
-    """Filter a ScryfallDataSet to include only specified set types."""
-    rejected_setcodes = set()
-    accepted_sets = []
-    for set_ in scryfall_data.sets:
-        if set_.set_type in set_types:
-            accepted_sets.append(set_)
-        else:
-            rejected_setcodes.add(set_.code)
-    accepted_cards = [c for c in scryfall_data.cards if c.set not in rejected_setcodes]
+    accepted_sets = [
+        s
+        for s in scryfall_data.sets
+        if s.code in accepted_setcodes and s.code in nonempty_setcodes
+    ]
     return ScryfallDataSet(sets=accepted_sets, cards=accepted_cards)

mtg_ssm/ssm.py

@@ -12,7 +12,7 @@
 from mtg_ssm.containers.collection import MagicCollection
 from mtg_ssm.containers.indexes import Oracle
 from mtg_ssm.scryfall import fetcher
-from mtg_ssm.scryfall.models import ScrySetType
+from mtg_ssm.scryfall.models import ScryCardLayout, ScrySetType
 
 
 def epilog() -> str:
@@ -32,13 +32,28 @@ def set_type_list(value: str) -> Set[ScrySetType]:
             set_types.add(ScrySetType(set_str))
         except ValueError as err:
             msg = (
-                f"{set_str} in {value} is not a valid set_type, please use a commas separated list of values from: "
+                f"{set_str} in {value} is not a valid set_type, please use a comma separated list of values from: "
                 + ", ".join(ScrySetType)
             )
             raise argparse.ArgumentTypeError(msg) from err
     return set_types
 
 
+def card_layout_list(value: str) -> Set[ScryCardLayout]:
+    """Argparse type to convert a string to a set of Scryfall Card Layouts."""
+    card_layouts = set()
+    for layout_str in value.split(","):
+        try:
+            card_layouts.add(ScryCardLayout(layout_str))
+        except ValueError as err:
+            msg = (
+                f"{layout_str} in {value} is not a valid layout_type, please use a comma separated list of values from: "
+                + ", ".join(ScryCardLayout)
+            )
+            raise argparse.ArgumentTypeError(msg) from err
+    return card_layouts
+
+
 def get_args(args: Optional[List[str]] = None) -> argparse.Namespace:
     """Parse and return application arguments."""
     parser = argparse.ArgumentParser(
@@ -66,15 +81,29 @@ def get_args(args: Optional[List[str]] = None) -> argparse.Namespace:
         "May be repeated for multiple different extensions.",
     )
 
-    default_set_types = set(ScrySetType) - {ScrySetType.MEMORABILIA, ScrySetType.TOKEN}
+    default_exclude_set_types = {ScrySetType.MEMORABILIA, ScrySetType.TOKEN}
     parser.add_argument(
-        "--set-types",
-        default=",".join(default_set_types),
+        "--exclude-set-types",
+        default=",".join(default_exclude_set_types),
         type=set_type_list,
-        help="List of set types to include as a comma separted list of values from: "
+        help="List of set types to exclude from data as a comma separated list of values from: "
         + ", ".join(ScrySetType),
     )
 
+    default_exclude_card_layouts = {
+        ScryCardLayout.ART_SERIES,
+        ScryCardLayout.DOUBLE_FACED_TOKEN,
+        ScryCardLayout.EMBLEM,
+        ScryCardLayout.TOKEN,
+    }
+    parser.add_argument(
+        "--exclude-card-layouts",
+        default=",".join(default_exclude_card_layouts),
+        type=card_layout_list,
+        help="List of card layouts to exclude from data as a comma separated list of values from: "
+        + ", ".join(ScryCardLayout),
+    )
+
     # Commands
     subparsers = parser.add_subparsers(dest="action", title="actions")
     subparsers.required = True
@@ -136,12 +165,20 @@ def get_args(args: Optional[List[str]] = None) -> argparse.Namespace:
     return parsed_args
 
 
-def get_oracle(set_types: Set[ScrySetType], include_digital: bool) -> Oracle:
+def get_oracle(
+    *,
+    exclude_set_types: Set[ScrySetType],
+    exclude_card_layouts: Set[ScryCardLayout],
+    include_digital: bool,
+) -> Oracle:
     """Get a card_db with current mtgjson data."""
     scrydata = fetcher.scryfetch()
-    scrydata = bundles.filter_set_types(scrydata, set_types)
-    if not include_digital:
-        scrydata = bundles.remove_digital(scrydata)
+    scrydata = bundles.filter_cards_and_sets(
+        scrydata,
+        exclude_set_types=exclude_set_types,
+        exclude_card_layouts=exclude_card_layouts,
+        exclude_digital=not include_digital,
+    )
     return Oracle(scrydata)
 
 
@@ -228,7 +265,11 @@ def diff_cmd(args: argparse.Namespace, oracle: Oracle) -> None:
 def main() -> None:
     """Get args and run the appropriate command."""
     args = get_args()
-    oracle = get_oracle(args.set_types, args.include_digital)
+    oracle = get_oracle(
+        exclude_set_types=args.exclude_set_types,
+        exclude_card_layouts=args.exclude_card_layouts,
+        include_digital=args.include_digital,
+    )
     args.func(args, oracle)
 
 

tests/containers/test_bundles.py

@@ -3,6 +3,7 @@
 from uuid import UUID
 
 from mtg_ssm.containers import bundles
+from mtg_ssm.scryfall.models import ScryCardLayout
 
 
 def test_remove_digital(scryfall_data: bundles.ScryfallDataSet) -> None:
@@ -17,7 +18,7 @@ def test_remove_digital(scryfall_data: bundles.ScryfallDataSet) -> None:
     assert "Cosmos Elixir" in card_names
     assert "A-Cosmos Elixir" in card_names
 
-    digital_removed = bundles.remove_digital(scryfall_data)
+    digital_removed = bundles.filter_cards_and_sets(scryfall_data, exclude_digital=True)
     set_codes2 = {s.code for s in digital_removed.sets}
     card_ids2 = {c.id for c in digital_removed.cards}
     card_names2 = {c.name for c in digital_removed.cards}
@@ -28,3 +29,19 @@ def test_remove_digital(scryfall_data: bundles.ScryfallDataSet) -> None:
     assert "khm" in set_codes2
     assert "Cosmos Elixir" in card_names2
     assert "A-Cosmos Elixir" not in card_names2
+
+
+def test_exclude_token_layout(scryfall_data: bundles.ScryfallDataSet) -> None:
+    set_codes = {s.code for s in scryfall_data.sets}
+    card_names = {c.name for c in scryfall_data.cards}
+
+    assert "p03" in set_codes and "sld" in set_codes
+    assert "Goblin" in card_names
+
+    tokens_removed = bundles.filter_cards_and_sets(
+        scryfall_data, exclude_card_layouts={ScryCardLayout.TOKEN}
+    )
+    set_codes2 = {s.code for s in tokens_removed.sets}
+    card_names2 = {c.name for c in tokens_removed.cards}
+    assert "p03" not in set_codes2 and "sld" in set_codes2
+    assert "Goblin" not in card_names2