-
Notifications
You must be signed in to change notification settings - Fork 9
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
Electrochemistry terms michael goette #55
Electrochemistry terms michael goette #55
Conversation
b7d589b
to
ffce27c
Compare
The current failure is due to a detail of #49 which takes a bit of time to fix. I'll look into it later. It is not caused by your xlsx file.
Because you control your range of IDs you could already use the IDs. In voc4cat they are only after merging of the PR,
What is the copyright? |
@RoteKekse I fixed the problem by updating the action to a new version of voc4cat-tool (0.8.2). You accidentally committed a temporary office file that I removed ( My updates re-started CI and it successfully converted your xlsx to turtle files and removed the xlsx file in an auto-commit (22a4558). If you need xlsx again to further edit your terms you can get a new xlsx file from the workflow artifacts. Click on the green check-mark left of the commit hash (def449c), then "details", then go to the summary of the job, and download the artifact at the bottom. The artifact is a zip file with re-built xlsx, RDF/turtle, and HTML-documentation. |
22a4558
to
eeee46c
Compare
@dalito i updated from the source but i think i can find the excel in an older commit, i think it might be better only delete it if the CI is run on the main branch in the main repo |
Hey @dalito i took the description know from here. https://knowledge.electrochem.org/ed/dict.htm# I would used links but i think these resources are not persistent. I put in most terms i saw in our lab in electrocatalysis, let me know what you think. |
@dalito i know there is likely a lot to discuss but this is what i am using for the electrochemistry in OER in nomad. Having term IDs for these things would be great and we would be happy to test it :). |
OK. We will have a look and be not too picky about minor details (which could be improved in follow-up PRs). |
The hierarchy is reversed: "final potential"(7217) has child (skos:narrower) "electric potential" (7219) - I looks like you consequently have it reversed (not just for these two concepts). |
@RoteKekse, thank you very much for your contribution. Please see below a few corrections needed, and some suggestions / comments to consider:
|
Hey @dalito thats true i intuitively recorded the parents because then you have at most one. I am sorry will correct it. |
Thanks @nmoust for the feedback. I get from your feedback, that in principle it looks good? |
I added the comments and reversed the |
f6db001
to
5b9302f
Compare
7df9d29
to
cbdac11
Compare
i inclued some comments from @schumannj |
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.
@RoteKekse Thanks for updates. A few points are left, see comments.
8a8ddd6
to
d58743d
Compare
3701b4f
to
7ba8686
Compare
…ber of voltammetry cycles
b54366a
to
d37be69
Compare
Yes. Just let me know if you want to update the electrical potential definition or if I should do it directly in ttl. @RoteKekse I pushed the update. |
@RoteKekse - Thank you for the contribution and your perseverance in the review process! |
Thank you for doing it. This is really valuable to us. :) we will include the iri in nomad in the upcoming weeks :) |
Hey i am still working on the terms, but since i have some questions and comments i created already a pull request.
i took definition so far mostly from here: https://echem101.gamry.com/electrochemical-terms/
when do i get ids assigned? only when the pull request is finished or do i have to set them by hand in the excel file?
further what is with qualities such as current, electric potential, current density etc. should we put them in?