Merge pull request #16 from Stepland/remove-null-values

[malody] Dumping does not write placeholder `null` values anymore

CHANGELOG.md

@@ -1,3 +1,10 @@
+# v1.2.2
+## Changed
+- Slashes in filenames are now ignored
+## Fixed
+- Fix bug that when using braces in filenames
+- [malody] Dumping does not write placeholder `null` values anymore
+
 # v1.2.1
 ## Fixed
 - [malody] Parsing a file with keys that are unused for conversion

jubeatools/formats/dump_tools.py

@@ -6,6 +6,7 @@ from typing import AbstractSet, Any, Dict, Iterator, TypedDict
 from jubeatools.formats.filetypes import ChartFile
 from jubeatools.formats.typing import ChartFileDumper, Dumper
 from jubeatools.song import Difficulty, Song
+from jubeatools.utils import none_or
 
 DIFFICULTY_NUMBER: Dict[str, int] = {
     Difficulty.BASIC: 1,
@@ -75,22 +76,42 @@ class FormatParameters(TypedDict, total=False):
     # uppercase BSC ADV EXT
     difficulty: str
     # 0-based
-    difficulty_index: int
+    difficulty_index: str
     # 1-based
-    difficulty_number: int
+    difficulty_number: str
     dedup: str
 
 
 def extract_format_params(chartfile: ChartFile, dedup_index: int) -> FormatParameters:
     return FormatParameters(
-        title=chartfile.song.metadata.title or "",
+        title=none_or(slugify, chartfile.song.metadata.title) or "",
-        difficulty=chartfile.difficulty,
+        difficulty=slugify(chartfile.difficulty),
-        difficulty_index=DIFFICULTY_INDEX.get(chartfile.difficulty, 3),
+        difficulty_index=str(DIFFICULTY_INDEX.get(chartfile.difficulty, 2)),
-        difficulty_number=DIFFICULTY_NUMBER.get(chartfile.difficulty, 4),
+        difficulty_number=str(DIFFICULTY_NUMBER.get(chartfile.difficulty, 3)),
         dedup="" if dedup_index == 0 else f"-{dedup_index}",
     )
 
 
+def slugify(s: str) -> str:
+    s = remove_slashes(s)
+    s = double_braces(s)
+    return s
+
+
+SLASHES = str.maketrans({"/": "", "\\": ""})
+
+
+def remove_slashes(s: str) -> str:
+    return s.translate(SLASHES)
+
+
+BRACES = str.maketrans({"{": "{{", "}": "}}"})
+
+
+def double_braces(s: str) -> str:
+    return s.translate(BRACES)
+
+
 class BetterStringFormatter(string.Formatter):
     """Enables the use of 'u' and 'l' suffixes in string format specifiers to
     convert the string to uppercase or lowercase

jubeatools/formats/jubeat_analyser/tests/memo/test_memo.py

@@ -13,7 +13,7 @@ from jubeatools.formats.jubeat_analyser.memo.load import MemoParser
 from jubeatools.testutils import strategies as jbst
 from jubeatools.testutils.test_patterns import dump_and_load_then_compare
 
-from ..test_utils import memo_compatible_song, temp_file_named_txt
+from ..test_utils import memo_compatible_song
 from . import example1, example2, example3
 
 
@@ -48,7 +48,6 @@ def test_that_full_chart_roundtrips(song: song.Song, circle_free: bool) -> None:
     dump_and_load_then_compare(
         Format.MEMO,
         song,
-        temp_path=temp_file_named_txt(),
         bytes_decoder=lambda b: b.decode("shift-jis-2004", errors="surrogateescape"),
         dump_options={"circle_free": circle_free},
     )

jubeatools/formats/jubeat_analyser/tests/memo1/test_memo1.py

@@ -12,7 +12,7 @@ from jubeatools.formats.jubeat_analyser.memo1.load import Memo1Parser
 from jubeatools.testutils.strategies import notes as notes_strat
 from jubeatools.testutils.test_patterns import dump_and_load_then_compare
 
-from ..test_utils import memo_compatible_song, temp_file_named_txt
+from ..test_utils import memo_compatible_song
 from . import example1
 
 
@@ -44,7 +44,6 @@ def test_that_full_chart_roundtrips(song: song.Song, circle_free: bool) -> None:
     dump_and_load_then_compare(
         Format.MEMO_1,
         song,
-        temp_path=temp_file_named_txt(),
         bytes_decoder=lambda b: b.decode("shift-jis-2004", errors="surrogateescape"),
         dump_options={"circle_free": circle_free},
     )

jubeatools/formats/jubeat_analyser/tests/memo2/test_memo2.py

@@ -22,7 +22,7 @@ from jubeatools.song import (
 from jubeatools.testutils.strategies import notes as notes_strat
 from jubeatools.testutils.test_patterns import dump_and_load_then_compare
 
-from ..test_utils import memo_compatible_song, temp_file_named_txt
+from ..test_utils import memo_compatible_song
 from . import example1, example2, example3
 
 
@@ -55,7 +55,6 @@ def test_that_full_chart_roundtrips(song: Song, circle_free: bool) -> None:
     dump_and_load_then_compare(
         Format.MEMO_2,
         song,
-        temp_path=temp_file_named_txt(),
         bytes_decoder=lambda b: b.decode("shift-jis-2004", errors="surrogateescape"),
         dump_options={"circle_free": circle_free},
     )

jubeatools/formats/jubeat_analyser/tests/mono_column/test_mono_column_hypothesis.py

@@ -24,7 +24,7 @@ from jubeatools.testutils.strategies import notes as notes_strat
 from jubeatools.testutils.strategies import tap_note
 from jubeatools.testutils.test_patterns import dump_and_load_then_compare
 
-from ..test_utils import memo_compatible_song, temp_file_named_txt
+from ..test_utils import memo_compatible_song
 
 
 @given(st.sets(tap_note(), min_size=1, max_size=100))
@@ -88,7 +88,6 @@ def test_that_full_chart_roundtrips(song: Song, circle_free: bool) -> None:
     dump_and_load_then_compare(
         Format.MONO_COLUMN,
         song,
-        temp_path=temp_file_named_txt(),
         bytes_decoder=lambda b: b.decode("shift-jis-2004", errors="surrogateescape"),
         dump_options={"circle_free": circle_free},
     )

jubeatools/formats/jubeat_analyser/tests/test_utils.py

@@ -12,7 +12,26 @@ from jubeatools.testutils.typing import DrawFunc
 
 @st.composite
 def memo_compatible_metadata(draw: DrawFunc) -> song.Metadata:
-    text_strat = st.text(alphabet=st.characters(min_codepoint=0x20, max_codepoint=0x7E))
+    # some ranges that are valid in shift-jis
+    text_strat = st.text(
+        alphabet=st.one_of(
+            *(
+                st.characters(min_codepoint=a, max_codepoint=b)
+                for a, b in (
+                    (0x20, 0x7F),
+                    (0xB6, 0x109),
+                    (0x410, 0x44F),
+                    (0x24D0, 0x24E9),
+                    (0x3041, 0x3096),
+                    (0x309B, 0x30FF),
+                    (0xFA30, 0xFA6A),
+                    (0xFF01, 0xFF3B),
+                    (0xFF3D, 0xFF5D),
+                    (0xFF61, 0xFF9F),
+                )
+            )
+        )
+    )
     metadata: song.Metadata = draw(
         jbst.metadata(text_strat=text_strat, path_strat=text_strat)
     )

jubeatools/formats/konami/eve/tests/test_eve.py

@@ -2,7 +2,7 @@ from hypothesis import given
 
 from jubeatools import song
 from jubeatools.formats import Format
-from jubeatools.formats.konami.testutils import eve_compatible_song, open_temp_dir
+from jubeatools.formats.konami.testutils import eve_compatible_song
 from jubeatools.testutils.test_patterns import dump_and_load_then_compare
 
 
@@ -11,7 +11,6 @@ def test_that_full_chart_roundtrips(song: song.Song) -> None:
     dump_and_load_then_compare(
         Format.EVE,
         song,
-        temp_path=open_temp_dir(),
         bytes_decoder=lambda b: b.decode("ascii"),
         load_options={"beat_snap": 12},
     )

jubeatools/formats/konami/jbsq/test_jbsq.py

@@ -2,7 +2,7 @@ from hypothesis import given
 
 from jubeatools import song
 from jubeatools.formats import Format
-from jubeatools.formats.konami.testutils import eve_compatible_song, open_temp_dir
+from jubeatools.formats.konami.testutils import eve_compatible_song
 from jubeatools.testutils.test_patterns import dump_and_load_then_compare
 
 from .construct import jbsq
@@ -13,7 +13,6 @@ def test_that_full_chart_roundtrips(song: song.Song) -> None:
     dump_and_load_then_compare(
         Format.JBSQ,
         song,
-        temp_path=open_temp_dir(),
         bytes_decoder=lambda b: str(jbsq.parse(b)),
         load_options={"beat_snap": 12},
     )

jubeatools/formats/konami/testutils.py

@@ -1,8 +1,4 @@
-import tempfile
-from contextlib import contextmanager
 from decimal import Decimal
-from pathlib import Path
-from typing import Iterator
 
 from hypothesis import strategies as st
 
@@ -53,9 +49,3 @@ def eve_compatible_song(draw: DrawFunc) -> song.Song:
         metadata=song.Metadata(),
         charts={diff: chart},
     )
-
-
-@contextmanager
-def open_temp_dir() -> Iterator[Path]:
-    with tempfile.TemporaryDirectory() as temp_dir:
-        yield Path(temp_dir)

jubeatools/formats/malody/dump.py

@@ -1,7 +1,7 @@
 import time
 from functools import singledispatch
 from pathlib import Path
-from typing import List, Tuple, Union
+from typing import List, Optional, Tuple, Union
 
 import simplejson as json
 
@@ -30,7 +30,7 @@ dump_malody = make_dumper_from_chart_file_dumper(
 
 
 def dump_malody_chart(
-    metadata: song.Metadata, dif: str, chart: song.Chart, timing: song.Timing
+    metadata: song.Metadata, dif: Optional[str], chart: song.Chart, timing: song.Timing
 ) -> malody.Chart:
     meta = dump_metadata(metadata, dif)
     time = dump_timing(timing)
@@ -40,10 +40,10 @@ def dump_malody_chart(
     return malody.Chart(meta=meta, time=time, note=notes)
 
 
-def dump_metadata(metadata: song.Metadata, dif: str) -> malody.Metadata:
+def dump_metadata(metadata: song.Metadata, dif: Optional[str]) -> malody.Metadata:
     return malody.Metadata(
-        cover="",
+        cover=None,
-        creator="",
+        creator=None,
         background=none_or(str, metadata.cover),
         version=dif,
         id=0,

jubeatools/formats/malody/schema.py

@@ -1,21 +1,15 @@
 from dataclasses import dataclass, field
 from decimal import Decimal
 from enum import Enum
-from typing import List, Optional, Tuple, Union
+from typing import Any, List, Optional, Tuple, Union
 
-from marshmallow import EXCLUDE
+from marshmallow import EXCLUDE, Schema, post_dump
 from marshmallow.validate import Range
 from marshmallow_dataclass import NewType, class_schema
 
 
-class Ordered:
-    class Meta:
-        ordered = True
-        unknown = EXCLUDE
-
-
 @dataclass
-class SongInfo(Ordered):
+class SongInfo:
     title: Optional[str]
     artist: Optional[str]
     id: Optional[int]
@@ -32,7 +26,7 @@ class Mode(int, Enum):
 
 
 @dataclass
-class Metadata(Ordered):
+class Metadata:
     cover: Optional[str]  # path to album art ?
     creator: Optional[str]  # Chart author
     background: Optional[str]  # path to background image
@@ -51,7 +45,7 @@ StrictlyPositiveDecimal = NewType(
 
 
 @dataclass
-class BPMEvent(Ordered):
+class BPMEvent:
     beat: BeatTime
     bpm: StrictlyPositiveDecimal
 
@@ -60,13 +54,13 @@ ButtonIndex = NewType("ButtonIndex", int, validate=Range(min=0, max=15))
 
 
 @dataclass
-class TapNote(Ordered):
+class TapNote:
     beat: BeatTime
     index: ButtonIndex
 
 
 @dataclass
-class LongNote(Ordered):
+class LongNote:
     beat: BeatTime
     index: ButtonIndex
     endbeat: BeatTime
@@ -74,7 +68,7 @@ class LongNote(Ordered):
 
 
 @dataclass
-class Sound(Ordered):
+class Sound:
     """Used both for the background music and keysounds"""
 
     beat: BeatTime
@@ -95,10 +89,20 @@ Event = Union[Sound, LongNote, TapNote]
 
 
 @dataclass
-class Chart(Ordered):
+class Chart:
     meta: Metadata
     time: List[BPMEvent] = field(default_factory=list)
     note: List[Event] = field(default_factory=list)
 
 
-CHART_SCHEMA = class_schema(Chart)()
+class BaseSchema(Schema):
+    class Meta:
+        ordered = True
+        unknown = EXCLUDE
+
+    @post_dump
+    def remove_none_values(self, data: dict, **kwargs: Any) -> dict:
+        return {key: value for key, value in data.items() if value is not None}
+
+
+CHART_SCHEMA = class_schema(Chart, base_schema=BaseSchema)()

jubeatools/formats/malody/tests/test_malody.py

@@ -1,32 +1,83 @@
+from dataclasses import fields
 from decimal import Decimal
+from typing import Optional
 
+import simplejson as json
 from hypothesis import given
 from hypothesis import strategies as st
 
 from jubeatools import song
 from jubeatools.formats import Format
-from jubeatools.formats.konami.testutils import open_temp_dir
+from jubeatools.formats.malody import schema as malody
+from jubeatools.formats.malody.dump import dump_malody_chart
 from jubeatools.testutils import strategies as jbst
 from jubeatools.testutils.test_patterns import dump_and_load_then_compare
 from jubeatools.testutils.typing import DrawFunc
 
 
 @st.composite
-def malody_compatible_song(draw: DrawFunc) -> song.Song:
+def difficulty(draw: DrawFunc) -> str:
-    """Malody files only hold one chart and have limited metadata"""
+    d: song.Difficulty = draw(st.sampled_from(list(song.Difficulty)))
-    diff = draw(st.sampled_from(list(song.Difficulty))).value
+    return d.value
-    chart = draw(jbst.chart(level_strat=st.just(Decimal(0))))
+
-    metadata = draw(jbst.metadata())
+
+@st.composite
+def chart(draw: DrawFunc) -> song.Chart:
+    c: song.Chart = draw(jbst.chart(level_strat=st.just(Decimal(0))))
+    return c
+
+
+@st.composite
+def metadata(draw: DrawFunc) -> song.Metadata:
+    metadata: song.Metadata = draw(jbst.metadata())
     metadata.preview = None
     metadata.preview_file = None
-    return song.Song(metadata=metadata, charts={diff: chart})
+    return metadata
 
 
-@given(malody_compatible_song())
+@st.composite
-def test_that_full_chart_roundtrips(song: song.Song) -> None:
+def malody_song(draw: DrawFunc) -> song.Song:
+    """Malody files only hold one chart and have limited metadata"""
+    diff = draw(difficulty())
+    chart_ = draw(chart())
+    metadata_ = draw(metadata())
+    return song.Song(metadata=metadata_, charts={diff: chart_})
+
+
+@given(malody_song())
+def test_that_full_chart_roundtrips(s: song.Song) -> None:
     dump_and_load_then_compare(
         Format.MALODY,
-        song,
+        s,
-        temp_path=open_temp_dir(),
         bytes_decoder=lambda b: b.decode("utf-8"),
     )
+
+
+@given(chart(), metadata(), st.one_of(st.none(), difficulty()))
+def test_that_none_values_in_metadata_dont_appear_in_dumped_json(
+    chart: song.Chart,
+    metadata: song.Metadata,
+    dif: Optional[str],
+) -> None:
+    assert chart.timing is not None
+    malody_chart = dump_malody_chart(metadata, dif, chart, chart.timing)
+    json_chart = malody.CHART_SCHEMA.dump(malody_chart)
+    assert all(value is not None for value in json_chart["meta"].values())
+
+
+@given(malody_song())
+def test_that_field_are_ordered(s: song.Song) -> None:
+    dif, chart = next(iter(s.charts.items()))
+    assert chart.timing is not None
+    malody_chart = dump_malody_chart(s.metadata, dif, chart, chart.timing)
+    json_chart = malody.CHART_SCHEMA.dump(malody_chart)
+    text_chart = json.dumps(json_chart, indent=4, use_decimal=True)
+    reparsed_chart = json.loads(
+        text_chart,
+    )
+    # dict is ordered in 3.8 ... right ?
+    order_in_file = list(reparsed_chart["meta"].keys())
+    order_in_definition = list(
+        f.name for f in fields(malody.Metadata) if f.name in reparsed_chart["meta"]
+    )
+    assert order_in_file == order_in_definition

jubeatools/testutils/test_patterns.py

@@ -1,5 +1,7 @@
+import tempfile
+from contextlib import contextmanager
 from pathlib import Path
-from typing import Callable, ContextManager, Optional
+from typing import Callable, ContextManager, Iterator, Optional
 
 from hypothesis import note
 
@@ -9,11 +11,17 @@ from jubeatools.formats.enum import Format
 from jubeatools.formats.guess import guess_format
 
 
+@contextmanager
+def open_temp_dir() -> Iterator[Path]:
+    with tempfile.TemporaryDirectory() as temp_dir:
+        yield Path(temp_dir)
+
+
 def dump_and_load_then_compare(
     format_: Format,
     song: song.Song,
-    temp_path: ContextManager[Path],
     bytes_decoder: Callable[[bytes], str],
+    temp_path: Callable[[], ContextManager[Path]] = open_temp_dir,
     load_options: Optional[dict] = None,
     dump_options: Optional[dict] = None,
 ) -> None:
@@ -21,11 +29,11 @@ def dump_and_load_then_compare(
     dump_options = dump_options or {}
     loader = LOADERS[format_]
     dumper = DUMPERS[format_]
-    with temp_path as path:
+    with temp_path() as folder_path:
-        files = dumper(song, path, **dump_options)
+        files = dumper(song, folder_path, **dump_options)
-        for path, bytes_ in files.items():
+        for file_path, bytes_ in files.items():
-            path.write_bytes(bytes_)
+            file_path.write_bytes(bytes_)
-            note(f"Wrote to {path} :\n{bytes_decoder(bytes_)}")
+            note(f"Wrote to {file_path} :\n{bytes_decoder(bytes_)}")
-            assert guess_format(path) == format_
+            assert guess_format(file_path) == format_
-        recovered_song = loader(path, **load_options)
+        recovered_song = loader(folder_path, **load_options)
         assert recovered_song == song