🔊 Better catching exceptions for graphviz install

felix0097 commented 1 month ago

I've pushed another fix @sunnyosun. The problem was that the display() call does not raise an exception but rather just prints the exception to the console.

The problem gets fixed if I call u._repr_mimebundle_() manually: display(u._repr_mimebundle_(), raw=True). Then the ExecutableNotFound exception gets raised + handled properly.

I've checked it on my laptop, now everything works as expected. What do you think?

Zethson commented 1 month ago

@felix0097 we should avoid calling underscore methods (private API that can change at any time) from external dependencies if possible. Is there an alternative?

felix0097 commented 1 month ago

I've looked into this @Zethson, I could only find the underscore version of the method

Ipython looks for any _repr_*_ method to display an object. See custom methods. So, not sure whether you can get around this. But I guess the API should be fairly stable then?

sunnyosun commented 1 month ago

Thank you @felix0097! Let's merge this for now and keep an eye on API change.

codecov[bot] commented 1 month ago

Codecov Report

Attention: Patch coverage is 66.66667% with 1 lines in your changes are missing coverage. Please review.

Project coverage is 89.59%. Comparing base (18f3374) to head (70575ac). Report is 14 commits behind head on main.

Files	Patch %	Lines
lamindb/_parents.py	66.66%	1 Missing :warning:

Additional details and impacted files

```diff @@ Coverage Diff @@ ## main #1679 +/- ## ========================================== - Coverage 90.00% 89.59% -0.41% ========================================== Files 51 51 Lines 5442 5644 +202 ========================================== + Hits 4898 5057 +159 - Misses 544 587 +43 ```

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

laminlabs / lamindb

🔊 Better catching exceptions for graphviz install #1679

Codecov Report