VSinerva / markov-music-generator

1 stars 0 forks source link

Vertaisarviointi #1

Open rikurauhala opened 2 years ago

rikurauhala commented 2 years ago

Vertaisarviointi

Lähdekoodi haettu GitHubista keskiviikkona 5. lokakuuta 2022 kello 19:20. Koodikatselmointi suoritettu noin kahden tunnin aikana samana iltana.

Yleistä

Projektin aihe vaikuttaa mielenkiintoiselta, joskin monimutkaiselta näin aiheeseen perehtymättömän näkökulmasta. Minulta kului alkuun jonkin verran aikaa saada sovellus suoritettua, sillä repositoriosta ei löytynyt asennusohjeita. Sain kuitenkin sovelluksen ajettua luomalla uuden virtuaaliympäristön ja asentamalla riippuvuudet komennolla pip install -r requirements.txt. Suosittelen kuitenkin lisäämään asennusohjeet repositorioon seuraavaa vertaisarviointia varten! Dokumentaation puolella on kuitenkin selitetty itse sovelluksen toiminta varsin hyvin, mikä on nähdäkseni tärkeämpää vertaisarvoinnin kannalta.

En ole ehkä paras henkilö arvioimaan tätä työtä, sillä aihe on itselleni vieras enkä juuri mitään musiikin tuottamisestakaan ymmärrä.

Ohjelman toimivuus

Ohjelma vaikuttaa toimivan enimmäkseen odotetulla tavalla enkä ainakaan saanut sitä kaatumaan.

Voisiko parametreihin kenties lisätä validointia? Esimerkiksi tempolle on varmaankin olemassa jotkin järkevät rajat, nyt sille voi antaa arvoksi lähes minkä tahansa kokonaisluvun, myös negatiivisen. Tähän liittyen, sain ohjelman toimimaan odottamattomalla tavalla antamalla parametreille hassuja arvoja ja yrittämällä generoida nuotteja.

Generoitiin 2 nuottia!

KOMENNON MUOTO VÄÄRÄ!

Opetusdata: 'opetusdata/*.mid'
Generoidun sävelmän polku: 'savelma.mid'

Ohjelma siis ilmoittaa samaan aikaan tehneensä jotakin, mutta myös komennon ("g") muodon olevan väärä.

Toinen ongelmatapaus on se, että opetusdataa ei ole saatavilla sillä sitä ei löydy repositoriosta, eikä dokumentaatio myöskään tarjoa ohjeita opetusdatan hankkimista varten. Lisääthän ohjeet näihin liittyen! Luin kylläkin kommenttisi viikkoraportista ja ymmärrän tekijänoikeuksiin liittyvät kysymykset. Myös midi-tiedostojen avaamiseen ja tarkasteluun kannattaa lisätä ohjeet, sillä formaatti ei varmasti ole monelle tuttu.

Koodin laatu

Koodin laatu on varsin hyvää ja Python ohjelmointikielenä on selvästi hallussa. Funktiot ja muuttujat on nimetty järkevästi ja projektin rakenne on looginen.

Suosittelisin kuitenkin käyttämään .pylintrc-tiedostoa enemmän hyväksi. Voit esimerkiksi ottaa pohjaksi tiedoston oletusasetuksilla pylint --generate-rcfile > .pylintrc ja muokata sitä omaan ohjelmaasi sopivaksi. Ainakin omasta mielestäni tämä on järkevämpi tapa kuin pylint-sääntöjen kytkeminen pois päältä rivi/tapauskohtaisesti. Toki kyseessä on jossain määrin myös makuasia!

Testit

Testit vaikuttavat testaavan järkeviä asioita. Kaikki testit menevät läpi komentoriviltä ajettaessa. Midi-käsittelijään voisi varmasti kirjoittaa lisääkin testejä, mutta uskon tämän olevan sinulla jo työlistalla.

Yhteenveto

Hienoa työtä! Projektisi vaikuttaa todella mielenkiintoiselta, vaikka en sen toimintaa aivan kokonaan vielä ymmärtänytkään. Toivottavasti palautteestani on sinulle apua.

VSinerva commented 2 years ago

Kiitos palautteesta!

Kirjoitin valikon alunperin sillä ajatuksella, että se helpottaa omaa elämääni ohjelman testaamisessa ja esittelemisessä, tästä johtuu puutteet ohjeissa ja syötteiden validoinnissa. Arvostan kuitenkin parannusehdotuksia ja toteutankin ne seuraavaa versiota varten :)

MIDI-käsittelijän testaamista laajennan vielä sen verran, että lisään luettavaan tiedostoon asioita, jotka ohjelman tulisi sivuuttaa oikein, ja väärässä sävellajissa olevia nuotteja. Tämän laajempaa testaamista en pidä tarpeellisena tässä vaiheessa, sillä varsinaisen tiedostojen käsittelyn hoitaa kuitenkin luotettava ulkopuolinen kirjasto. Myös kurssin materiaalissa on mainittu tiedostojen lukeminen ja kirjoittaminen asioina, jotka voi jättää yksikkötestien ulkopuolelle.

Kirjoitan myös ohjeet opetusdatan hankkimiseen ja yleisesti MIDI-tiedostojen käsittelyyn. Pyrin myös varmentumaan käyttämäni datan lisensseistä niin, että voisin laittaa ne repositorioon.

Kiitos vinkistä pylintin suhteen! Luonkin samantien kattavamman tiedoston. Yksittäisen rivin tasolla virheiden poistaminen on kuitenkin mielestäni hyvä tapa poiketa yleisestä "best practices"-tyyppisistä ohjeista nimenomaan tapauskohtaisesti. Tämä toimintatapa vaatii harkitsemaan erikseen joka kerta, kun haluaa poiketa ohjeesta, sen sijaan että poistaisi virheen koko projektin tasolla kerran.

Pahoittelen vielä turhaa päänvaivaa jota puuttelliset ohjeet tuottivat.