-
Notifications
You must be signed in to change notification settings - Fork 35
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add West Old Turkic concept list #1240
Conversation
Add new concepts from [West Old Turkic](#1180) -- soon to be added to concepticon
remove extra tab in last line
The devil is in the details: The semantic field tag needs to be changed from "Agriculture and Vegetation" to "Agriculture and vegetation". I only see the file |
Okay strange, my GitHub desktop didn't push the changes, now the 5 Files are modified. Sorry for the blooper. Will correct the "V" zu "v" in a bit. |
From the few interactions I had with GitHub desktop, I found it not very helpful, in particular when it comes to diagnosing bugs/unexpected situations. Using git on the command line might make this more transparent. |
Three more issues:
One question:
|
So now I have:
Getting following error message when running INFO concepticon/concepticon-data at /home/viktor/Documents/GitHub/concepticon-data |
|
Thanks for the quick help! INFO concepticon/concepticon-data at /home/viktor/Documents/GitHub/concepticon-data |
Looks like the software does what it's supposed to do: pointing out problems with the data. Do you think the reported problem is a false positive? |
I'm having troubles understanding what the error message is trying to tell me. What I so far understand is that some file somewhere in the repository is not a string or byte-like object. But which one that could be? There are three tsv-files, one json, and one .bib |
That's a bit tricky to debug, I admit. But |
and implemented rest of the recommendations.
Cool, thanks for the help! Re-defined INFO concepticon/concepticon-data at /home/viktor/Documents/GitHub/concepticon-data |
Ok, great! It looks like the PR is ready for our usual review process of checking the mappings and metadata. @mathildavz and @Tarotis Could you be a reviewer for this PR? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'll happily take over one of the review. Thank you @martino-vic for adding this concept list. Many times I find the mapping difficult because of the blurry meanings involved with the historical data.This is why I would thus propose to unmap in some cases, as you can see in my comments. I also had doubts on two cases, where I tagged @AnnikaTjuka to provide guidance.
removed 1972 KID once and replaced it once with YOUNG GOAT (KID)
Many thanks for the review, @Tarotis! I left some comments. @martino-vic In some cases, it would be helpful if you could give us a bit more detail about the list to disambiguate the mappings. |
removed the empty line on the bottom (#1240 (comment))
in accordance with #1240 (comment)
@martino-vic Could you please run the |
Changed the description: Removed "Cuman", since this was resolved in LoanDB/ronataswestoldturkic#13 and mentioned the author of the original source, for clarity.
Had testing 410 concept lists |
Nice, then merge, once you got approved and green light from the others,
many thanks!
|
@Tarotis Could you check that all the changes you requested have been implemented and approve the PR if so? I'll do a final check afterward. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I added a small amount of comments, primarily with questions and some comments on the bib-file :)
Thank you for the helpful comments @Tarotis. I implemented the recommendations during the last PR. The tests seem to be passing too. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for all the work put into this, green light from my side now :)
replaced the comma with the dot before the subtitle
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for your patience and for resolving all comments, @martino-vic! Feel free to merge now. And congratulations on submitting your first list!
I just merged, so @martino-vic can concentrate on the blogpost, which is important for the dissertation ;-) Thanks to all of you for the patience, great help, and enthusiasm to discuss so many details! |
Thanks everybody for the great cooperation! |
Pull request checklist
concepticon notlinked --gloss "NEW_GLOSS"
Additional information
I've followed the instructions mentioned in this issue
I didn't add new Concepticon concept sets, as that was the outcome of this issue
I'm not sure what the last four bullets mean, would be glad if somebody could give me a pointer
Also, when I run
conception test
I getValueError: invalid Conceptset.semanticfield: ['Agriculture and Vegetation']
, I couldn't decipher the rest of the error message either, which is:INFO concepticon/concepticon-data at /home/viktor/Documents/GitHub/concepticon-data
testing 410 concept lists
Traceback (most recent call last):
File "/home/viktor/Documents/cldfvenv3.9/bin/concepticon", line 8, in
sys.exit(main())
File "/home/viktor/Documents/cldfvenv3.9/lib/python3.9/site-packages/pyconcepticon/main.py", line 62, in main
return args.main(args) or 0
File "/home/viktor/Documents/cldfvenv3.9/lib/python3.9/site-packages/pyconcepticon/commands/test.py", line 21, in run
if args.repos.check(*args.clids): # pragma: no cover
File "/home/viktor/Documents/cldfvenv3.9/lib/python3.9/site-packages/pyconcepticon/api.py", line 391, in check
'concepticon_id': set(self.conceptsets.keys()),
File "/home/viktor/Documents/cldfvenv3.9/lib/python3.9/site-packages/clldutils/misc.py", line 197, in get
result = instance.dict[self.name] = self.fget(instance)
File "/home/viktor/Documents/cldfvenv3.9/lib/python3.9/site-packages/pyconcepticon/api.py", line 120, in conceptsets
return to_dict(
File "/home/viktor/Documents/cldfvenv3.9/lib/python3.9/site-packages/pyconcepticon/util.py", line 36, in to_dict
for obj in iterobjects:
File "/home/viktor/Documents/cldfvenv3.9/lib/python3.9/site-packages/pyconcepticon/api.py", line 121, in
Conceptset(api=self, **lowercase(d))
File "", line 10, in init
File "/home/viktor/Documents/cldfvenv3.9/lib/python3.9/site-packages/pyconcepticon/models.py", line 69, in valid_key
raise ValueError('invalid {0}.{1}: {2}'.format(
ValueError: invalid Conceptset.semanticfield: ['Agriculture and Vegetation']
...