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

rework missing import mechanism? #193

Open
baggiponte opened this issue Apr 16, 2024 · 2 comments
Open

rework missing import mechanism? #193

baggiponte opened this issue Apr 16, 2024 · 2 comments
Assignees
Labels
refactor Code change that neither fixes a bug nor adds a feature

Comments

@baggiponte
Copy link
Collaborator

baggiponte commented Apr 16, 2024

Currently we deal with ImportErrors like this:

try:
    from .lance import ann
except ImportError:
    msg = "Missing ann extras: `pip install functime[ann]`"
    ann = ImportError(msg)

try:
    from .automl import auto_lightgbm
    from .lightgbm import flaml_lightgbm, lightgbm
except ImportError:
    msg = "Missing lightgbm extras: `pip install functime[lgb]`"
    auto_lightgbm = ImportError(msg)
    flaml_lightgbm = ImportError(msg)
    lightgbm = ImportError(msg)

try:
    from .catboost import catboost
except ImportError:
    catboost = ImportError("Missing catboost extras: `pip install functime[cat]`")

try:
    from .xgboost import xgboost
except ImportError:
    xgboost = ImportError("Missing xgboost extras: `pip install functime[xgb]`")

This is clever but means that the issue is raised "lazily" and can lead to issues such as #190 . I think we should revert this.

@baggiponte baggiponte self-assigned this Apr 16, 2024
@baggiponte baggiponte added bug Something isn't working refactor Code change that neither fixes a bug nor adds a feature and removed bug Something isn't working labels Apr 16, 2024
@baggiponte baggiponte changed the title rework missing import mechanism rework missing import mechanism? Apr 16, 2024
@FBruzzesi
Copy link
Contributor

In scikit-lego we have a little more sophisticated version of such error which can help to point at which version to install (proof).

In general there is a tradeoff between:

  • what the user has immediately available
  • the number of packages required to be installed

Personally, as the base/core dependencies of the functime are already quite broad, I would not add them all 😇 my two cents

@baggiponte
Copy link
Collaborator Author

+1 on that. I would actually like to remove as many dependencies as I can (I am looking at you, cloudpickle). I need to improve on the lazy imports though. Polars does too.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
refactor Code change that neither fixes a bug nor adds a feature
Projects
None yet
Development

No branches or pull requests

2 participants