Merge pull request #18 from Stepland/type-error-missing-beat-snap-17

Avoid shadowing loader/dumper default values for options

CHANGELOG.md

@@ -1,3 +1,9 @@
+# Unreleased
+## Fixed
+- Loaders and Dumpers would recieve options with unwanted default values when
+  their corresponding flags were not passed to the commandline, resulting
+  in weird bugs, not anymore ! #17
+
 # v1.2.2
 ## Changed
 - Slashes in filenames are now ignored

jubeatools/cli/cli.py

@@ -50,8 +50,8 @@ def convert(
     dst: str,
     input_format: Optional[Format],
     output_format: Format,
-    loader_options: Dict[str, Any],
-    dumper_options: Dict[str, Any],
+    loader_options: Optional[Dict[str, Any]] = None,
+    dumper_options: Optional[Dict[str, Any]] = None,
 ) -> None:
     """Convert SRC to DST using the format specified by -f"""
     if input_format is None:
@@ -68,7 +68,9 @@ def convert(
     except KeyError:
         raise ValueError(f"Unsupported output format : {input_format}")
 
+    loader_options = loader_options or {}
     song = loader(Path(src), **loader_options)
+    dumper_options = dumper_options or {}
     files = dumper(song, Path(dst), **dumper_options)
     for path, contents in files.items():
         with path.open("wb") as f:

jubeatools/cli/helpers.py

@@ -3,17 +3,6 @@ from typing import Any, Callable, Union
 import click
 
 
-def add_to_dict(
-    key: str,
-) -> Callable[[click.Context, Union[click.Option, click.Parameter], Any], None]:
-    def add_to_key(
-        ctx: click.Context, param: Union[click.Option, click.Parameter], value: Any
-    ) -> None:
-        ctx.params.setdefault(key, {})[param.name] = value
-
-    return add_to_key
-
-
 def loader_option(*args: Any, **kwargs: Any) -> Callable:
     return click.option(
         *args, callback=add_to_dict("loader_options"), expose_value=False, **kwargs
@@ -24,3 +13,28 @@ def dumper_option(*args: Any, **kwargs: Any) -> Callable:
     return click.option(
         *args, callback=add_to_dict("dumper_options"), expose_value=False, **kwargs
     )
+
+
+def add_to_dict(
+    key: str,
+) -> Callable[[click.Context, Union[click.Option, click.Parameter], Any], None]:
+    def add_to_key(
+        ctx: click.Context, param: Union[click.Option, click.Parameter], value: Any
+    ) -> None:
+        # Avoid shadowing load/dump functions kwargs default values with the
+        # default values chosen by click
+        assert param.name is not None
+        if not parameter_is_a_click_default(ctx, param.name):
+            ctx.params.setdefault(key, {})[param.name] = value
+
+    return add_to_key
+
+
+def parameter_is_a_click_default(
+    ctx: click.Context,
+    name: str,
+) -> bool:
+    return ctx.get_parameter_source(name) in (
+        click.core.ParameterSource.DEFAULT,
+        click.core.ParameterSource.DEFAULT_MAP,
+    )

jubeatools/cli/tests/data/__init__.py

@@ -0,0 +1,2 @@
+"""This file is here so the test code can use importlib as a portable way to
+open test data in this folder"""

jubeatools/cli/tests/test_cli.py

@@ -0,0 +1,20 @@
+from importlib import resources
+
+from click.testing import CliRunner
+
+from ..cli import convert
+from . import data
+
+
+def test_that_ommiting_beat_snap_works() -> None:
+    """
+    As pointed out by https://github.com/Stepland/jubeatools/issues/17
+    """
+    runner = CliRunner()
+    with runner.isolated_filesystem(), resources.path(
+        data, "Life Without You.eve"
+    ) as p:
+        result = runner.invoke(
+            convert, [str(p.resolve(strict=True)), "out.txt", "-f", "memo2"]
+        )
+        assert result.exit_code == 0

jubeatools/testutils/strategies.py

@@ -69,7 +69,8 @@ def note_position(draw: st.DrawFn) -> NotePosition:
 
 @st.composite
 def tap_note(
-    draw: st.DrawFn, time_strat: st.SearchStrategy[BeatsTime] = beat_time(max_section=10)
+    draw: st.DrawFn,
+    time_strat: st.SearchStrategy[BeatsTime] = beat_time(max_section=10),
 ) -> TapNote:
     time = draw(time_strat)
     position = draw(note_position())