Conversation
There was a problem hiding this comment.
Pull request overview
Updates PytoTune’s public-facing APIs (C++ + Python + CLI) to use new casing/names while expanding documentation across headers and the README.
Changes:
- Renames core tuning entrypoints (
tuneToMidi→matchMidi,tuneToScale→roundToScale) and updates callers (CLI/tests/bindings). - Renames multiple Python binding symbols (functions,
Scalemethods/properties/kwargs) to camelCase. - Adds/rewrites docstrings and significantly expands
README.md.
Reviewed changes
Copilot reviewed 16 out of 16 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
| tests/test_api.cpp | Updates tests to call renamed C++ API functions. |
| test_bindings.py | Updates example script to use renamed Python API symbols. |
| src/python_bindings.cpp | Renames exposed Python symbols and Scale API surface in pybind11 bindings. |
| src/cli.cpp | Updates CLI to call renamed C++ API functions. |
| src/api.cpp | Renames C++ API function definitions to match new names. |
| include/pytotune/io/wav_file.h | Docstring rewrite for WAV structures/classes. |
| include/pytotune/io/midi_file.h | Adds richer MIDI parsing/API documentation. |
| include/pytotune/data-structures/windowing.h | Adds docs for windowing and windowed data helpers. |
| include/pytotune/data-structures/scale.h | Removes file header banner comment. |
| include/pytotune/data-structures/mode.h | Removes file header banner comment. |
| include/pytotune/api.h | Renames the public C++ API declarations. |
| include/pytotune/algorithms/yin_pitch_detector.h | Adds/updates docs for PitchRange, VoiceRanges, and detector API. |
| include/pytotune/algorithms/pitch_shifter.h | Adds/updates class/method documentation. |
| include/pytotune/algorithms/pitch_correction_pipeline.h | Adds/updates pipeline API documentation. |
| include/pytotune/algorithms/fft.h | Minor docstring tweak / header cleanup. |
| README.md | Replaces minimal README with full usage/build/docs (reflecting new API names). |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| std::string midiFile = constants::TEST_DATA_DIR + "/test.mid"; | ||
| std::string wavFile = constants::VOICE_F400_SR4100; | ||
| std::string outputFile = constants::TEST_OUTPUT_DIR + "/test_tune_to_midi.wav"; | ||
|
|
There was a problem hiding this comment.
The test names were updated to MatchMidi/RoundToScale, but the output filenames still use the old test_tune_to_* naming. Consider renaming these output paths to match the new API names to keep test artifacts easier to interpret.
| m.def("tuneToScale", &p2t::tuneToScale, "Tune a WAV file to a musical scale", | ||
| m.def("roundToScale", &p2t::roundToScale, "Tune a WAV file to a musical scale", | ||
| py::arg("wav_path"), py::arg("scale"), py::arg("out_path"), | ||
| py::arg("pitch_range") = p2t::VoiceRanges::HUMAN); |
There was a problem hiding this comment.
Renaming the Python module functions from tuneToMidi/tuneToScale to matchMidi/roundToScale is a breaking change for existing Python users. Consider keeping the old names as aliases (possibly marked deprecated) that forward to the new functions so existing scripts keep working.
| py::arg("pitch_range") = p2t::VoiceRanges::HUMAN); | |
| py::arg("pitch_range") = p2t::VoiceRanges::HUMAN); | |
| // Backwards-compatible aliases for older API names (deprecated in favor of matchMidi/roundToScale) | |
| m.def("tuneToMidi", &p2t::matchMidi, "[DEPRECATED] Use matchMidi instead.", | |
| py::arg("wav_path"), py::arg("midi_path"), py::arg("out_path"), | |
| py::arg("pitch_range") = p2t::VoiceRanges::HUMAN); | |
| m.def("tuneToScale", &p2t::roundToScale, "[DEPRECATED] Use roundToScale instead.", | |
| py::arg("wav_path"), py::arg("scale"), py::arg("out_path"), | |
| py::arg("pitch_range") = p2t::VoiceRanges::HUMAN); |
No description provided.