[malody] Dumping does not write placeholder null values anymore

CHANGELOG.md

@@ -1,3 +1,7 @@
+# v1.2.2
+## Fixed
+- [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/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/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,85 @@
+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

@@ -21,11 +21,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