codecov-commenter
commented
8 months ago Codecov Report
Attention: 4 lines in your changes are missing coverage. Please review.
Comparison is base (
55ba0ce) 74.39% compared to head (e8baeb1) 74.36%.
| Files | Patch % | Lines |
|---|---|---|
| kgforge/core/forge.py | 42.85% | 4 Missing :warning: |
Additional details and impacted files
```diff @@ Coverage Diff @@ ## master #362 +/- ## ========================================== - Coverage 74.39% 74.36% -0.04% ========================================== Files 100 100 Lines 6307 6315 +8 ========================================== + Hits 4692 4696 +4 - Misses 1615 1619 +4 ``` | [Flag](https://app.codecov.io/gh/BlueBrain/nexus-forge/pull/362/flags?src=pr&el=flags&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=BlueBrain) | Coverage Δ | | |---|---|---| | [unittests](https://app.codecov.io/gh/BlueBrain/nexus-forge/pull/362/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=BlueBrain) | `74.36% <92.15%> (-0.04%)` | :arrow_down: | Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=BlueBrain#carryforward-flags-in-the-pull-request-comment) to find out more.:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.
KnowledgeGraphForge picks specializations of mappings/mappers as default types of Mapping/Mapper to use. There's a cyclic dependency error if one tries to import both KnowledgeGraphForge and DictionaryMapping.
This removes the imports of DictionaryMapping and DictionaryMapper in KnowledgeGraphForge: I think having core import specializations is not a good practice, and I've replaced the default value that KnowledgeGraphForge with the mapping and mapper defined by the store it holds. It could be that this makes no sense, if ever the intended usage of these mappings/mappers is different.
I've also made the specializations file import the classes directly from the files they were defined, and not the packages they are contained in. I feel like it's more of a user interface feature to have the ability to import from packages. It could be argued that specializations should be defined by users and so they should also import the same way...