search

ICB-DCM / pyPESTO

python Parameter EStimation TOolbox
https://pypesto.readthedocs.io
BSD 3-Clause "New" or "Revised" License
207 stars 45 forks source link

Petab import in try/except clause that fails when amici is not available #1340

Closed m-philipps closed 3 months ago

m-philipps commented 3 months ago

petab is a necessary import for using the petab.PetabImporter, yet it is imported within a try/except clause, and after the import of amici. That means that when amici is not available, petab can not be imported.

https://github.com/ICB-DCM/pyPESTO/blob/34e89b3bc88d2052ca808da43c11c57da75bec04/pypesto/petab/importer.py#L54-L69

Is there a reason to use try/except around the petab import for the petab.importer?

PaulJonasJost commented 3 months ago

I would not see a specific reason to keep it in the try except clause. pypesto.petab has to be imported separately and at this stage I think we can expect people to have installed petab. What do you think @dweindl, @Doresic?

Doresic commented 3 months ago

Yeah, think it can be moved outside. A thought: I remember I had to do this for my ordinal/semiquantitative stuff even though they should only ever be used with those two included. If I didn't do it, some tests that do not install petab/amici would fail. However, I was always surprised by how and why these tests would import those modules then. Back then I didn't understand it much / think twice. But there might be something weird there.

PaulJonasJost commented 3 months ago

closed in #1355