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.

Sign up for GitHub

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

Jump to bottom

Feature/82-NLtoAPI-Implement-basic-entity-linker #89

Merged
merged 25 commits into from
Feb 2, 2024
Merged

Feature/82-NLtoAPI-Implement-basic-entity-linker #89

merged 25 commits into from
Feb 2, 2024

Conversation

IKostric
Copy link
Contributor

Adds simple entity linker - dbpedia spotlight.

@IKostric IKostric requested review from NoB0 and WerLaj as code owners January 30, 2024 13:45
@github-actions GitHub Actions
Copy link

github-actions bot commented Jan 30, 2024 •
edited
Loading

Unit Test Results

47 tests   47 ✔️  4s ⏱️
  1 suites    0 💤
  1 files      0 ❌

Results for commit 808cb95.

♻️ This comment has been updated with latest results.

@github-actions GitHub Actions
Copy link

github-actions bot commented Jan 30, 2024 •
edited
Loading

Coverage Report •
FileStmtsMissCoverMissing
pkg_api/nl_to_pkg
   nl_to_pkg.py120100% 
pkg_api/nl_to_pkg/entity_linking
   entity_linker.py6183%25
   spotlight_entity_linker.py34385%37, 41–>39, 61, 93
pkg_api/util
   load_config.py11273%23, 26
tests/nl_to_pkg
   test_nl_to_pkg.py470100% 
   test_spotlight_entity_linker.py430100% 
TOTAL7873594% 

@github-actions GitHub Actions
Copy link

Current Branch Main Branch
Coverage Badge Coverage Badge

IKostric added 12 commits January 31, 2024 05:43
@IKostric
Add TripleElement class for SPO
9fa8adc
@IKostric
Merge branch 'feature/87-Simplify-Triple-Only-use-Concept-instead-of-…
da9abb1 
…Unionstr-Concept' of https://github.com/iai-group/pkg-api into feature/82-NLtoAPI-Implement-basic-entity-linker
@IKostric
Migrate to new dataclass structure
3694cc8
@IKostric
Add linking for predicate and object
dce50f6
@IKostric
Address review comments
0309b5c
@IKostric
Merge branch 'main' of https://github.com/iai-group/pkg-api into feat…
9d565ef 
…ure/87-Simplify-Triple-Only-use-Concept-instead-of-Unionstr-Concept
@IKostric
fix imports
df3d2c9
@IKostric
Fix pkg inserts with new dataclass structure
75a0d29
@IKostric
[NLtoAPI] Implement basic entity linker
26bf78e 
Fixes #82
@IKostric
Merge branch 'main' of https://github.com/iai-group/pkg-api into feat…
6cf7d05 
…ure/82-NLtoAPI-Implement-basic-entity-linker
@IKostric
Swap value and reference
10e7e47
@IKostric
Merge branch 'main' of https://github.com/iai-group/pkg-api into feat…
4ebd07d 
…ure/82-NLtoAPI-Implement-basic-entity-linker
NoB0
NoB0 requested changes Feb 1, 2024
View reviewed changes
Copy link
Collaborator

@NoB0 NoB0 left a comment •
edited
Loading

Choose a reason for hiding this comment

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

See inline comments and please fix pre-commit errors.

pkg_api/nl_to_pkg/entity_linking/spotlight_entity_linker.py Outdated Show resolved Hide resolved
pkg_api/nl_to_pkg/entity_linking/spotlight_entity_linker.py Show resolved Hide resolved
pkg_api/nl_to_pkg/entity_linking/spotlight_entity_linker.py Outdated Show resolved Hide resolved
tests/nl_to_pkg/test_spotlight_entity_linker.py Outdated Show resolved Hide resolved
tests/nl_to_pkg/test_spotlight_entity_linker.py Outdated Show resolved Hide resolved
@IKostric IKostric requested a review from NoB0 February 1, 2024 09:48
NoB0
NoB0 approved these changes Feb 1, 2024
View reviewed changes
Copy link
Collaborator

@NoB0 NoB0 left a comment

Choose a reason for hiding this comment

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

LGTM. Just some nits to look. Please make sure that the CI checks are successful before merging.

pkg_api/nl_to_pkg/entity_linking/spotlight_entity_linker.py Show resolved Hide resolved
pkg_api/nl_to_pkg/entity_linking/spotlight_entity_linker.py Outdated Show resolved Hide resolved
WerLaj
WerLaj approved these changes Feb 1, 2024
View reviewed changes
Copy link
Collaborator

@WerLaj WerLaj left a comment

Choose a reason for hiding this comment

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

LGTM but some clarification is needed at some points

pkg_api/nl_to_pkg/entity_linking/spotlight_entity_linker.py Outdated Show resolved Hide resolved
pkg_api/nl_to_pkg/entity_linking/spotlight_entity_linker.py Show resolved Hide resolved
pkg_api/nl_to_pkg/entity_linking/spotlight_entity_linker.py Show resolved Hide resolved
@IKostric IKostric merged commit cda83a0 into main Feb 2, 2024
5 checks passed
@IKostric IKostric deleted the feature/82-NLtoAPI-Implement-basic-entity-linker branch February 2, 2024 06:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@NoB0 NoB0 NoB0 approved these changes

@WerLaj WerLaj WerLaj approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@IKostric @WerLaj @NoB0