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

Misleading Error "Please install necessary libs for CatBoostModel." #1187

Closed
you-n-g opened this issue Jul 8, 2022 · 3 comments
Closed

Misleading Error "Please install necessary libs for CatBoostModel." #1187

you-n-g opened this issue Jul 8, 2022 · 3 comments
Labels
bug Something isn't working good first issue Good for newcomers

Comments

@you-n-g
Copy link
Collaborator

you-n-g commented Jul 8, 2022

🐛 Bug Description

Qlib does not require the installation of packages like CatBoostModel

But the output looks a little misleading.

To Reproduce

Run examples/workflow_by_code.ipynb in jupyter notebook.

Expected Behavior

Successfully run the script without installing CatBoostModel and warning.

Screenshot

image

@you-n-g you-n-g added bug Something isn't working good first issue Good for newcomers labels Jul 8, 2022
@Derek-Wds
Copy link
Contributor

Derek-Wds commented Jul 8, 2022

Maybe we could do something like:

class CatBoostModel:
    def __init__(self):
        try:
            from catboost import ***
        except ModuleNotFoundError:
            print("Warning.")

But I feel like this is not elegant enough and may be regarded as invalid in some format style like PEP8.
Here could be an alternative:
image

@OussCHE
Copy link
Contributor

OussCHE commented Jul 24, 2022

I've debugged the code , it is triggered if catboost is not installed. We can modify the warning to be similar to all other warnings (like when LGBModel or double_ensemble are not available)

I am still new to the library , so any advice would be appreciated.

Edit : path to the screen shot https://github.com/microsoft/qlib/blob/main/qlib/contrib/model/__init__.py
error_location

@you-n-g
Copy link
Collaborator Author

you-n-g commented Aug 5, 2022

I think this language support may be useful to make this module cleaner.
We can have a unified implementation of __getattr__ to handle all kinds of imports.

https://stackoverflow.com/a/48916205

qianyun210603 pushed a commit to qianyun210603/qlib that referenced this issue Mar 23, 2023
…tModel." (microsoft#1246)

* Update __init__.py

* Update __init__.py
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working good first issue Good for newcomers
Projects
None yet
Development

No branches or pull requests

3 participants