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.

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

Model use - relative imports #54

Open
Hrovatin opened this issue Aug 14, 2024 · 4 comments
Open

Model use - relative imports #54

Hrovatin opened this issue Aug 14, 2024 · 4 comments

Comments

@Hrovatin
Copy link

I cloned the repo yesterday and tried to import from SaProt.model.saprot.base import SaprotBaseModel as per some of the code examples. This failed as the "utils" import in many of the model files was not relative. Would suggest changing it to e.g. from ...utils.metrics import count_f1_max and similar across model scripts.

@LTEnjoy
Copy link
Contributor

LTEnjoy commented Aug 14, 2024

Hi,

We have thought about it before but we thought it would make the relation among files to be more complicated and may cause some unexpected errors when your execution directory varies. A simple workaround is to add the root path of this project to sys.path, e.g.:

import sys
sys.path.append("/your/path/to/SaProt")

Then it should work well.

@Hrovatin Hrovatin changed the title Model use Model use - relative imports Aug 14, 2024
@Hrovatin
Copy link
Author

Hrovatin commented Aug 14, 2024

This is not ideal as then it may be unclear if utils should be loaded from SaProt or my own utils directory that is somewhere in the path
Best solution would be to just make it a python package so that everything can be imported as SaProt.utils etc

@LTEnjoy
Copy link
Contributor

LTEnjoy commented Aug 14, 2024

In real situation it hardly happens that two utils directories conflict as every file in SaProt will load specific functions defined in utils, e.g. utils.metrics import count_f1_max. This would lead the system to the true utils directory. I understand the best solution to pack the SaProt into a individual pacakage but given some coding logics in the repo it might be not easy to implement it. I will add a __init__.py file at the root path that will automatically add the root path to sys.path. This could resolve the import problem from outer directory.

@LTEnjoy
Copy link
Contributor

LTEnjoy commented Aug 14, 2024

We have already uploaded a __init__.py for outer import. You can clone the repo again and try it out:)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants