Skip to content

Conversation

@francescalb
Copy link
Collaborator

Description

Type of change

  • Bug fix.
  • New feature.
  • Documentation update.
  • Test 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

@francescalb francescalb changed the base branch from master to flb/emmo_1.0.0_released March 14, 2025 21:03
Copy link
Collaborator

@jesper-friis jesper-friis left a comment

Choose a reason for hiding this comment

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

See the comments.

The tests are failing, but I guess that the purpose of PR #839 is to fix them.

@francescalb francescalb marked this pull request as draft March 18, 2025 14:29
@francescalb
Copy link
Collaborator Author

This PR has never been ready for review. will close all comments for now.

See the comments.

The tests are failing, but I guess that the purpose of PR #839 is to fix them.

This PR has never been ready for review. will close all comments for now.

Base automatically changed from flb/emmo_1.0.0_released to master March 18, 2025 14:44
@francescalb francescalb changed the base branch from master to flb/specifiy_iri_new_entity March 20, 2025 08:12
Base automatically changed from flb/specifiy_iri_new_entity to master March 21, 2025 17:45
)
namespace = splitlabel[0]

#print('namespace', namespace)
Copy link
Collaborator

Choose a reason for hiding this comment

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

can be removed


#print('namespace', namespace)
if namespace:
#print('in get_by_label namespavce')
Copy link
Collaborator

Choose a reason for hiding this comment

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

can be removed

Comment on lines +522 to +528
#print("in namespace", namespace)
#print("entities", entities)
#for ent in entities:
# print("ent.namespace", ent.namespace)
# print("ent.namespace.name", ent.namespace.name)
# print("namespace", namespace)
# print(namespace in [ent.namespace.name, ent.namespace.base_iri])
Copy link
Collaborator

Choose a reason for hiding this comment

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

can be removed

Copy link
Collaborator

@jesper-friis jesper-friis left a comment

Choose a reason for hiding this comment

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

Tests are currently failing.

There is also a trivial merge-conflict and some print-statements that can be removed.

exact_match: Do not treat "*" and brackets as special characters
when matching. May be useful if your ontology has labels
containing such labels.
label: label so search for.
Copy link
Collaborator

Choose a reason for hiding this comment

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

looks like an extra indentation

return 2 * ccadepth / (generations1 + generations2 + 2 * ccadepth)

def new_entity(
def new_entity( # pylint: disable=too-many-arguments,too-many-branches,too-many-positional-arguments
Copy link
Collaborator

Choose a reason for hiding this comment

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

These pylint disable's will probably make the line too long. Probably better to move the comment to after the docstring (line 2119)

Copy link
Collaborator

@jesper-friis jesper-friis left a comment

Choose a reason for hiding this comment

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

There are some conflicts and failing tests that must be fixed.

I see that one of the tests fails with that emmo.hasPart doesn't exists. That should be there. Can there be something wrong in the changes in get_by_label()?

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.

3 participants