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

Fix internal links in generated documentation generated with ontodoc #548

Merged
merged 10 commits into from
Feb 12, 2023

Conversation

jesper-friis
Copy link
Collaborator

@jesper-friis jesper-friis commented Feb 7, 2023

Description:

PR #543 changed the id of documented concepts from preflabel to the name-part of the IRI. This PR updates internal links accordingly such that they works again.

The implementation became little more involved, because utils.asstring() now needs access to the ontology.

Type of change:

  • Bug fix.
  • New feature.
  • Documentation update.

Checklist:

This checklist can be used as a help for the reviewer.

  • Is the code easy to read and understand?
  • Are comments for humans to read, not computers to disregard?
  • Does a new feature has an accompanying new test (in the CI or unit testing schemes)?
  • Has the documentation been updated as necessary?
  • Does this close the issue?
  • Is the change limited to the issue?
  • Are errors handled for all outcomes?
  • Does the new feature provide new restrictions on dependencies, and if so is this documented?

Comments:

@codecov-commenter
Copy link

codecov-commenter commented Feb 7, 2023

Codecov Report

Merging #548 (ce5f4c8) into master (ac8c999) will decrease coverage by 0.04%.
The diff coverage is 79.31%.

📣 This organization is not using Codecov’s GitHub App Integration. We recommend you install it so Codecov can continue to function properly for your repositories. Learn more

@@            Coverage Diff             @@
##           master     #548      +/-   ##
==========================================
- Coverage   65.85%   65.82%   -0.04%     
==========================================
  Files          16       16              
  Lines        3099     3105       +6     
==========================================
+ Hits         2041     2044       +3     
- Misses       1058     1061       +3     
Impacted Files Coverage Δ
ontopy/utils.py 62.01% <75.00%> (-0.24%) ⬇️
ontopy/graph.py 61.63% <100.00%> (ø)
ontopy/ontodoc.py 67.51% <100.00%> (ø)
ontopy/ontology.py 70.27% <100.00%> (ø)

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

Copy link
Collaborator

@francescalb francescalb left a comment

Choose a reason for hiding this comment

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

This new version does not write out the iris correctly:
image

As you see, it give IRI: Entropy. The link is the IRI but written as prefLabel. This seems wrong to me. Also, didn't it used to be that one can click from one concept in the document to another? Now all clicking leads to the same https://raw.githubusercontent.com/emmo-repo/EMMO/master/emmo.ttl#IRI ( IRI being anythin as it is not in that file).

@francescalb francescalb removed the request for review from CasperWA February 10, 2023 14:30
asstring() now treats string arguments as strings. If possible, it tries
to derive IRI from the string argument.

Links now uses relative references by default. This makes links independent
of redirections and is much faster.  But is it really what we want?
@jesper-friis
Copy link
Collaborator Author

This new version does not write out the iris correctly: image

As you see, it give IRI: Entropy. The link is the IRI but written as prefLabel. This seems wrong to me. Also, didn't it used to be that one can click from one concept in the document to another? Now all clicking leads to the same https://raw.githubusercontent.com/emmo-repo/EMMO/master/emmo.ttl#IRI ( IRI being anythin as it is not in that file).

Very well seen! Thanks. That the IRIs ended up being shown with their prefLabel was a logical error, which is solved now.
The asstring() function now treats a string argument as the string to show. It still tries to be smart and see if it can infer an entity from the string argument, but that will only affect the link, not the shown value.

The other issue with the links was actually by design (as requested by BIG-MAP). However, the old behaviour with relative links make much more sense, especially before the redirections works properly. Hence, we are now generating relative links by default.

Copy link
Collaborator

@francescalb francescalb left a comment

Choose a reason for hiding this comment

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

Looks good.

@jesper-friis jesper-friis merged commit 7918322 into master Feb 12, 2023
@jesper-friis jesper-friis deleted the ontodoc-fix-links branch February 12, 2023 16:17
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.

4 participants