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

feat (documentation): declutter and enrich the datamodel graph #1504

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

gythaogg
Copy link
Contributor

  1. show only one edge between a subject model and an object model
  2. show reverse name of the relations in the edge label if it is different from the name
  3. slightly reduce the size of the label on the edge

image

TBD:

  • 1 & 2 above only implemented for new relations, can we drop support for apis_relations?
  • Since we are showing both forrward and reverse relations, should the edges be bidirectional?

@gythaogg gythaogg force-pushed the gythaogg/docs-relations branch from e2704da to 7730668 Compare December 18, 2024 09:50
@gythaogg gythaogg changed the title feat (documentation): declutter the datamodel graph feat (documentation): include reverse relations and declutter datamodel graph Dec 18, 2024
- show only one edge between a subject model and an object model
- show reverse name of the relations in the edge label if it is different
from the name
- slightly reduce the size of the label on the edge
@gythaogg gythaogg force-pushed the gythaogg/docs-relations branch from 7730668 to 1d602d1 Compare December 18, 2024 09:52
@gythaogg gythaogg changed the title feat (documentation): include reverse relations and declutter datamodel graph feat (documentation): declutter and enrich the datamodel graph Dec 18, 2024
@gythaogg gythaogg requested a review from b1rger December 18, 2024 09:54
Copy link
Contributor

@b1rger b1rger left a comment

Choose a reason for hiding this comment

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

Sorry for the delay.

Regarding the code: i think it makes sense to use force_str for all the name() methods, maybe do this once, assign the result to variables and then use them in the if/else statement.

Regarding functionality: I think its a feature to be able to distinguish between relations that go in one direction and relations that go in the other direction. The change does show me the reverse name, which is great, but I loose the information in which direction the relation goes (which might be important, because its not always possible to deduce this from the name/reverse_name)

@gythaogg
Copy link
Contributor Author

gythaogg commented Jan 23, 2025

Regarding the code: i think it makes sense to use force_str for all the name() methods, maybe do this once, assign the result to variables and then use them in the if/else statement.

Agreed - will do.

Regarding functionality: I think its a feature to be able to distinguish between relations that go in one direction and relations that go in the other direction. The change does show me the reverse name, which is great, but I loose the information in which direction the relation goes (which might be important, because its not always possible to deduce this from the name/reverse_name)

Yes, good point - I'll come up with a couple of options - maybe we can discuss it in the JF?

@b1rger
Copy link
Contributor

b1rger commented Jan 23, 2025

Yes, good point - I'll come up with a couple of options - maybe we can discuss it in the JF?

sure!

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.

2 participants