Skip to content
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 new Word content type #181

Merged
merged 8 commits into from
Nov 9, 2023
Merged

Add new Word content type #181

merged 8 commits into from
Nov 9, 2023

Conversation

CasperWA
Copy link
Contributor

@CasperWA CasperWA commented Oct 30, 2023

Closes #76

Adds Word in a similar fashion as Code and similar.


When storing the Turtle file (cif-ddl.ttl) from Protégé it changed the formatting. In order to understand the actual change(s), I have split it up into several commits.

To see the purely formatting changes, see a34fd90.
To see the addition of Word, see d47bf00.


Fixes #182 by enforcing at minimum v4.4.6 of PyCIFRW.


This PR goes a small step further and updates the cif-core.ttl file under ontologies/ to the latest generated version of the CIF-ontology according to the updated cif_core.dic in COMCIFS and the newly added Word content type.
Note, since the Word content type concept has not yet been approved by @emanueleghedini I'd not consider the generated version of the CIF-ontology 100 % accurate (yet).

Copy link
Collaborator

@vaitkus vaitkus left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The commit dealing with the addition of the Word content type seems ok to me.

@vaitkus
Copy link
Collaborator

vaitkus commented Oct 30, 2023

Ok, so I did some more proper testing and the issue described in #76 still manifests, but with a slightly different error message.

Command:

dic2owl cif_core.dic

Error:

    item = self.get_by_label(name)
  File "<virtualenv-dir>/cif-emmo/lib/python3.10/site-packages/ontopy/ontology.py", line 405, in get_by_label
    raise NoSuchLabelError(f"No label annotations matches '{label}'")
ontopy.utils.NoSuchLabelError: No label annotations matches 'Word'

Note, that I had to manually remove non-ASCII symbols from the downloaded dic/cif files to temporarily circumvent issue #182.

Copy link
Collaborator

@vaitkus vaitkus left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

See comment #181 (comment).

@CasperWA
Copy link
Contributor Author

Ok, so I did some more proper testing and the issue described in #76 still manifests, but with a slightly different error message.

It seems to me the issue is still the same - the concept cannot be found. I'll check a bit locally as well (didn't do this, just updated the ontology 😅 ).

@CasperWA
Copy link
Contributor Author

CasperWA commented Oct 31, 2023

As a note, I managed to make this work locally. I had to also manually revert non-ASCII characters in templ_attr.cif and templ_enum.cif, but could otherwise decode/encode the cif_core.dic auto-ignoring non-ASCII characters.

Furthermore, I've implemented a --local-ontologies flag for the dic2owl CLI, which will use the cif-ddl.ttl file under ontologies/.

However, this will not work properly until #182 has been fixed, so I'd consider this PR blocked by that issue.

The newly generated CIF Core ontology can be found here.

Fixes #182

Generate and update `cif-core.ttl` under ontologies.
@CasperWA CasperWA linked an issue Nov 1, 2023 that may be closed by this pull request
@CasperWA CasperWA requested a review from vaitkus November 1, 2023 08:19
@vaitkus
Copy link
Collaborator

vaitkus commented Nov 1, 2023

Version 4.4.6 of PyCIFRW resolves the Unicode issue (thanks @jamesrhester), however, the issue with Word seems to still be there. The Word issue does not manifest when running with the --local-ontologies option.

If it works properly for @CasperWA, this might just be an issue with me running things incorrectly. Commands I use in a new virtual environment with a fresh checkout of CIF-ontology branch cwa/add-Word branch under LinuxMint 21.2 (read Ubuntu 22.04):

cd ~/src/CIF-ontology/dic2owl
pip3 install wheel
pip3 install -e .
dic2owl cif_core.dic

@CasperWA
Copy link
Contributor Author

CasperWA commented Nov 1, 2023

(...) If it works properly for @CasperWA, this might just be an issue with me running things incorrectly. Commands I use in a new virtual environment with a fresh checkout of CIF-ontology branch cwa/add-Word branch under LinuxMint 21.2 (read Ubuntu 22.04):

cd ~/src/CIF-ontology/dic2owl
pip3 install wheel
pip3 install -e .
dic2owl cif_core.dic

You should run dic2owl --local-ontologies cif_core.dic in the last command there - then it should work 👍

EDIT: Also, to ensure you're not reusing any manually edited dependency CIF and dic files, you should remove any locally downloaded templ_attr.cif, templ_enum.cif, ddl.dic, cif_core.dic files

@vaitkus
Copy link
Collaborator

vaitkus commented Nov 1, 2023

@CasperWA Yes, as I have mentioned in my earlier comment (#181 (comment)), the command with --local-ontologies does not raise any warnings and works well. I just did not realize this is the new intended approach. I'll approve the PR.

@CasperWA
Copy link
Contributor Author

CasperWA commented Nov 1, 2023

@CasperWA Yes, as I have mentioned in my earlier comment (#181 (comment)), the command with --local-ontologies does not raise any warnings and works well. I just did not realize this is the new intended approach. I'll approve the PR.

Ah - it's not the new intended approach, it is merely a way to use the local version of the DDL ontology, which has the Word concept, and not the DDL ontology from the main branch (this will be used by default), which currently does not have the Word concept.

Sorry for the confusion. The --local-ontologies option is merely a temporary solution to check that the suggested implementation in the current PR actually works 😃

@CasperWA
Copy link
Contributor Author

CasperWA commented Nov 9, 2023

Have not gotten any response from @emanueleghedini. Will merge this, as it fixes CI workflows and will instead open an issue to follow-up the ontology consistency.

@CasperWA CasperWA merged commit 17281e9 into main Nov 9, 2023
1 check passed
@CasperWA CasperWA deleted the cwa/add-Word branch November 9, 2023 09:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Handle non-ASCII Unicode symbols in CIF dictionary files Add support for the new Word DDLm content type
2 participants