cldf-clts / pyclts

Apache License 2.0
11 stars 2 forks source link

Update to README.md #4

Closed tresoldi closed 4 years ago

tresoldi commented 5 years ago

This informs the correct command for downloading the catalog (catconfig and not config) and informs that pyglottolog and pyconcepticon are needed.

codecov-io commented 5 years ago

Codecov Report

Merging #4 into master will not change coverage. The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##           master       #4   +/-   ##
=======================================
  Coverage   94.67%   94.67%           
=======================================
  Files          26       26           
  Lines        1071     1071           
=======================================
  Hits         1014     1014           
  Misses         57       57

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 4842f1f...6e0a4ea. Read the comment docs.

xrotwang commented 5 years ago

Thanks for catching this! I'm not sure the additional requirements need to be mentioned, though. Doesn't cldfbench catconfig work without having the API installed?

tresoldi commented 5 years ago

I assumed it would, also from reading the code, but I was not able to get it working without it.

It could very well be my fault, but I should have mentioned that what triggered it was the running of tests on Travis. Maybe there is a side-dependency that ends up asking for them? I will check, and if you want more details, this is my travil.yml and of course here is the log.

But in hindsight, I suppose we don't need to mention pyconcepticon and pyglottolog even if they are indeed dependencies.

xrotwang commented 5 years ago

If this is only required for tests, then maybe adding these dependencies here https://github.com/cldf-clts/pyclts/blob/4842f1fd9613de6ef20a917dbc3bd723e8d0ffbb/setup.py#L35 would be the solution?

tresoldi commented 5 years ago

Sounds good. We only change config to catconfig in the README and add pyconcepticon and pyglottolog there.

chrzyki commented 4 years ago

Can this be merged?

xrotwang commented 4 years ago

yes.