Closed tresoldi closed 3 years ago
What is it that we are missing from LAPSyD? I thought everything was mapped there now.
@tresoldi, I cannot see this commit due to the file number. But tell me please quickly: did you add the new column which we call Symbols" to the tsv files in this run? If not, I suggest strongly to do this now, as it will otherwise again be a large PR. We later also modify pyclts to add the symbols as well in the pkg/-files.
@tresoldi, can you please answer my question on the Symbols? I'd like to know if this still needs to be advaanced or not? I mean, the CLTS commandd is already there, and routinely adds them. Just a quick reply please.
@LinguList I kept looking here and can't understand which "Symbols column" you mean. I followed your example as much as I could, and it was the same workflow for the mappings we already merged (i.e., phoible, lapsyd, jipa).
@tresoldi, I wrote an email on this topic, and @cormacanderson and you both agreed that the feature was useful. Please pull the most recent pyclts code, and run the mapping one time, to see what I actually mean.
I know, it is the email from 22 Nov 2020 20:10 and I agree it is a good idea. What I meant is that I cannot understand what I should have changed in the workflow.
For all sources, I ran clts map
first, manually proceeded in correcting/improving the mappings, generated each package with clts make_dataset
, and finally regenerated the app with clts make_app
.
I did this with a pyclts
from your map-fix
branch, up to commit https://github.com/cldf-clts/pyclts/commit/5d58dccb484f440f9438bc68469b0b6ca08cf1f9 which is the most recent I can see there.
I can revert the changes due to make_app
, and even due to all all make_dataset
, if you prefer to get a simpler diff, after addressing the points raised by @cormacanderson first.
Tiago, I cannot view the PR since it is mega large. Don't you see that? I ask you simply: do your files in our source folder HAVE the Systems column, or do they NOT have it. Can you please just answer on that question?
No, they don't have that column: https://github.com/cldf-clts/clts/blob/sources/sources/chomsky/graphemes.tsv
I ran clts map
again from my setup and it is not adding it, unfortunately.
I will redo the PR only with the various graphemes.tsv
, reverting all other files to the old versions.
I agree that the Symbols is a good idea.
However, if you just change the graphemes.tsv files in this PR, without making the necessary changes to pkg/transcriptionsystems/bipa/consonants.tsv will some of the graphemes.tsv mappings not fail?
Does it make sense to first make sure that all of the mappings are fine and then add in the Symbols in a new PR straight afterwards?
Yep, so my proposal is, having clarified this: you two, @cormacanderson and @tresoldi, figure out how to make sure all is correct, @tresoldi merges this PR, and then we fix with the symbols, but this time: step-by-step, not in the form of monster pull-requests, where too many files are modified.
So to finish this, I'd suggest to quickly address teh points made by @cormacanderson and then merge. We can later step by step do the conversion with the symbols for better visibility.
Ok, I will address the points by @cormacanderson , use the code that was merged into pyclts
, regenerate, and push.
Ok, I have updated the sources, addressed the issues, regenerated, and will now merge. Note that I did not touch allenbai
, beidasinitic
, and bdpa
.
This PR reviews the mapping for all sources (with the exception of
allenbai
,beidasinitic
, andbdpa
, which I did not touch), also adding a handful of new sounds to BIPA, such as "voiceless retroflex approximant", and regenerating all packages and the app.