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

[BUG] Addressing doc build issue due to failed soft dependency imports #2170

Merged
merged 12 commits into from Mar 7, 2022

Conversation

fkiraly
Copy link
Collaborator

@fkiraly fkiraly commented Mar 6, 2022

This PR is an attempt to address the doc build fails that @aiwalter had reported, i.e., empty/missing pages for estimators with soft dependencies. This was also reported earlier as #2013.

The way this is done is via a minor change to _check_soft_dependencies, which gets an additional argument severity that controls whether a warning is raised, or an exception.
The extended utility is then used as follows:

  • called on module import, but raises only a warning, so execution does not terminate
  • called additionally on class construction, with an exception.

That way, users as well as the doc build process can import classes, read docstrings, and will get an error only when the class is constructed. The informative error message is still raised at the earliest possible point in time.

This PR addresses the issue for (hopefully) all instances of _check_soft_dependencies preventing doc build previously.
It also contains changes to make the error message more informative.

Changes still need to be made to dependent docs that outline how implementers/developer should deal with soft dependencies: the developer documentation, and the extension templates.

This PR also fixes the following bug:

@fkiraly fkiraly added documentation Documentation & tutorials maintenance Continuous integration, unit testing & package distribution labels Mar 6, 2022
@fkiraly fkiraly added this to fixing in Bugfixing Mar 6, 2022
@fkiraly fkiraly requested a review from mloning as a code owner March 6, 2022 15:05
@fkiraly fkiraly linked an issue Mar 6, 2022 that may be closed by this pull request
@aiwalter aiwalter merged commit 72d658c into main Mar 7, 2022
Bugfixing automation moved this from fixing to Fixed/resolved Mar 7, 2022
@aiwalter aiwalter deleted the lazy_arima branch March 7, 2022 11:50
fkiraly added a commit that referenced this pull request Mar 9, 2022
This updates the docs on soft dependency handling with the workflow implied by the fix in #2170.

The main change is explaining that the check needs to be in two places, at the top of the module (warning), and in the estimator init (error).

Other, minor changes:
* restructuring of the "soft dependency" section, hopefully better to follow now
* removal of duplicate "deprecation" section, newer version is on the "deprecation" page
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation Documentation & tutorials maintenance Continuous integration, unit testing & package distribution
Projects
Bugfixing
Fixed/resolved
Development

Successfully merging this pull request may close these issues.

[BUG] forecasting API docs page is broken
2 participants