search

cldf-clts / pyclts

Apache License 2.0
11 stars 2 forks source link

update dataset #35

Closed LinguList closed 3 years ago

LinguList commented 3 years ago

@xrotwang, I am adding one more test for clts data here, which I consider important, as it allows to check if a given transcription dataset in the sources has been properly explicitly coded. This adds one more step to the workflow:

$ clts map sources/TD/graphemes.tsv
$ MANUAL EDITING
$ clts test_dataset TD
$ clts add_dataset TD

If an improper mapping is encountered (either not passing valid_sound or not yielding an unknown sound, this now also raises an error. The whole check routine for new transcription data was now added to one single function that is recycled in make_pkg which should only be used upon the final release. This yields more consistency, I hope.

codecov-io commented 3 years ago

Codecov Report

Merging #35 (efbcd36) into master (956b072) will increase coverage by 0.07%. The diff coverage is 77.77%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master      #35      +/-   ##
==========================================
+ Coverage   93.78%   93.85%   +0.07%     
==========================================
  Files          32       33       +1     
  Lines        1528     1563      +35     
==========================================
+ Hits         1433     1467      +34     
- Misses         95       96       +1
Impacted Files Coverage Δ
src/pyclts/commands/test_dataset.py 71.42% <71.42%> (ø)
src/pyclts/commands/make_dataset.py 76.47% <72.41%> (+48.81%) :arrow_up:
src/pyclts/commands/make_pkg.py 91.89% <100.00%> (+4.85%) :arrow_up:
src/pyclts/commands/map.py 42.69% <100.00%> (-16.86%) :arrow_down:
tests/test_cli.py 100.00% <100.00%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update 956b072...efbcd36. Read the comment docs.

LinguList commented 3 years ago

@xrotwang, I have no idea why the tests fail: they pass on my system, and the error message is all but clear to me...

xrotwang commented 3 years ago

The make_dataset command requires lingpy. So you'd need to add lingpy to the test deps in setup.py.

LinguList commented 3 years ago

Ah, okay, then I remove it for now. Sorry, I didn't really this.

xrotwang commented 3 years ago

But you can leave the test in - just add lingpy here: https://github.com/cldf-clts/pyclts/blob/956b07297b1402c39403b7c40e8f32d4eb24bae9/setup.py#L35-L40

LinguList commented 3 years ago

Yes, but it was stupid from me, since lingpy is not even required in these lines, so I removed them now, and we have the advantage of having quicker tests. It was any way onlly dead code that I added there...

LinguList commented 3 years ago

Okay, I just updated the test_data.tsv, which is now in concordance with the updates we made (including some changes in the features, and the like).