Made semsimian an optional dependency

[hrshdhgd]

Fixes #703

[codecov-commenter]

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 75.34%. Comparing base (388c78b) to head (efd8858).
Report is 1 commits behind head on main.

❗ Your organization needs to install the Codecov GitHub app to enable full functionality.

Additional details and impacted files

@@           Coverage Diff           @@
##             main     #704   +/-   ##
=======================================
  Coverage   75.34%   75.34%           
=======================================
  Files         266      266           
  Lines       31083    31083           
=======================================
  Hits        23420    23420           
  Misses       7663     7663           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[cmungall]

I think this might break things as-is

ontology-access-kit/src/oaklib/implementations/__init__.py

Lines 44 to 46 in 15bf85c

	from oaklib.implementations.semsimian.semsimian_implementation import (
	SemSimianImplementation,
	)

ontology-access-kit/src/oaklib/implementations/semsimian/semsimian_implementation.py

Line 9 in 15bf85c

from semsimian import Semsimian

[hrshdhgd]

I think this might break things as-is

ontology-access-kit/src/oaklib/implementations/__init__.py

Lines 44 to 46 in 15bf85c

	from oaklib.implementations.semsimian.semsimian_implementation import (
	SemSimianImplementation,
	)

ontology-access-kit/src/oaklib/implementations/semsimian/semsimian_implementation.py

Line 9 in 15bf85c

from semsimian import Semsimian

Aah! great catch ...that would have introduced more headaches. I think this is why we had semsimian as a required package. Thinking of a smart solution.....

[cmungall]

I think this is a bit complicated and will raise an error if someone imports semsimian_implementation. In general it should be safe to import any module in the current package.

Look at gilda_implementation for inspiration

[hrshdhgd]

Look at gilda_implementation for inspiration

You mean

if TYPE_CHECKING:
    from semsimian import Semsimian

Done.

[cmungall]

There should be no need for dynamic manipulation of __all__

This can be simplified further. When I say look at gilda, I mean make it completely parallel. It's completely analogous, gilda==semsimian. __init__.py still imports GildaImplementation. It's completely safe as gilda is only ever imported within a method that is explicitly executed by a client

[hrshdhgd]

Done

[cmungall]

sorry... still pretty confusing. So this is a global variable? With a leading uppercase that makes it sound like a class?

[hrshdhgd]

It is a class but it is instantiated as a None. The class only exists once it is imported from the semsimian package. I know it is super confusing. I still prefer my previous solution but this one works too. Mimicking Gilda comes with complications like this because we are caching the Semsimian object but Gilda does not cache anything. So something that works for Gilda implementation doesn't necessaarily seem to work for Semsimian implementation.

[cmungall]

You don't need to worry about caching. In fact this could be problematic. When you call get_adapter() it creates an SSI instance and this can hold on to the SS object

…

On Mon, Mar 11, 2024 at 8:49 PM Harshad ***@***.***> wrote: ***@***.**** commented on this pull request. ------------------------------ In src/oaklib/implementations/semsimian/semsimian_implementation.py <#704 (comment)> : > @@ -22,6 +20,8 @@ from oaklib.interfaces.semsim_interface import SemanticSimilarityInterface from oaklib.types import CURIE, PRED_CURIE +Semsimian: Optional["Semsimian"] = None # type: ignore It is a class but it is instantiated as a None. The class only exists once it is imported from the semsimian package. I know it is super confusing. I still prefer my previous solution but this one works too. Mimicking Gilda comes with complications like this because we are caching the Semsimian object but Gilda does not cache anything. So something that works for Gilda implementation doesn't necessaarily seem to work for Semsimian implementation. — Reply to this email directly, view it on GitHub <#704 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAAMMOKWI2ZSTV5QJS24PH3YX2CM3AVCNFSM6AAAAABEJ3XA22VHI2DSMVQWIX3LMV43YUDVNRWFEZLROVSXG5CSMV3GSZLXHMYTSMZQGAZTMMZYHA> . You are receiving this because your review was requested.Message ID: ***@***.***>