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

Pin to recent breathe, to prevent getting an unsupported sphinx version. #1495

Merged
merged 2 commits into from
Mar 6, 2024

Conversation

bdice
Copy link
Contributor

@bdice bdice commented Mar 5, 2024

Description

The RMM nightly docs builds failed. https://github.com/rapidsai/rmm/actions/runs/8151286975/job/22279173933#step:9:973

It ran into an error reported in sphinx-doc/sphinx#11490. It seems like a viable fix (that is also minimal and somewhat future-proof) is to pin to a recent version of breathe, which in turn forces a compatible version of sphinx. This is the same pinning used by cudf.

Checklist

  • I am familiar with the Contributing Guidelines.
  • New or existing tests cover these changes.
  • The documentation is up to date with these changes.

@github-actions github-actions bot added the conda label Mar 5, 2024
@bdice bdice added bug Something isn't working improvement Improvement / enhancement to an existing function labels Mar 5, 2024
@bdice bdice self-assigned this Mar 5, 2024
@bdice
Copy link
Contributor Author

bdice commented Mar 5, 2024

There are some warnings in CI that I don't know how to fix.

https://github.com/rapidsai/rmm/actions/runs/8158164146/job/22300050680?pr=1495#step:9:1013

 /__w/rmm/rmm/python/docs/docstring of rmm._lib.logger.logging_level:43: WARNING: py:obj reference target not found: denominator
/__w/rmm/rmm/python/docs/docstring of rmm._lib.logger.logging_level:46: WARNING: py:obj reference target not found: imag

/__w/rmm/rmm/python/docs/docstring of rmm._lib.logger.logging_level:49: WARNING: py:obj reference target not found: numerator
/__w/rmm/rmm/python/docs/docstring of rmm._lib.logger.logging_level:52: WARNING: py:obj reference target not found: real
/__w/rmm/rmm/python/docs/docstring of rmm._lib.logger.logging_level:64:<autosummary>:1: WARNING: py:obj reference target not found: rmm.logging_level.as_integer_ratio
/__w/rmm/rmm/python/docs/docstring of rmm._lib.logger.logging_level:64:<autosummary>:1: WARNING: py:obj reference target not found: rmm.logging_level.bit_count
/__w/rmm/rmm/python/docs/docstring of rmm._lib.logger.logging_level:64:<autosummary>:1: WARNING: py:obj reference target not found: rmm.logging_level.bit_length
/__w/rmm/rmm/python/docs/docstring of rmm._lib.logger.logging_level:64:<autosummary>:1: WARNING: py:obj reference target not found: rmm.logging_level.conjugate
/__w/rmm/rmm/python/docs/docstring of rmm._lib.logger.logging_level:64:<autosummary>:1: WARNING: py:obj reference target not found: rmm.logging_level.from_bytes
/__w/rmm/rmm/python/docs/docstring of rmm._lib.logger.logging_level:64:<autosummary>:1: WARNING: py:obj reference target not found: rmm.logging_level.to_bytes

@github-actions github-actions bot added the Python Related to RMM Python API label Mar 6, 2024
@vyasr vyasr added doc Documentation non-breaking Non-breaking change conda and removed bug Something isn't working conda improvement Improvement / enhancement to an existing function labels Mar 6, 2024
@bdice bdice marked this pull request as ready for review March 6, 2024 05:47
@bdice bdice requested review from a team as code owners March 6, 2024 05:47
Copy link
Member

@harrism harrism left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I like it. Thanks!

Comment on lines +212 to +217
python_names_to_skip = [x for x in dir(int) if not x.startswith("__")]
if (
node["refdomain"] == "py"
and (reftarget := node.get("reftarget")) is not None
and any(toskip in reftarget for toskip in python_names_to_skip)
):
Copy link
Member

@harrism harrism Mar 6, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nit: Could you just concatenate all the python names_to_skip that satisfy the refdomain/reftarget conditions with the C++ names to skip and have a single final any(toskip...) condition? Not sure if that would be more or less coherent.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There is a difference in node["refdomain"] == "py" and node["refdomain"] == "cpp". I think that's enough reason to separate these.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah, I missed the C++ side conditions because the any is inside the outer if on the Python side but in a nested one on the C++ side.

@bdice
Copy link
Contributor Author

bdice commented Mar 6, 2024

/merge

@rapids-bot rapids-bot bot merged commit 26a941b into rapidsai:branch-24.04 Mar 6, 2024
53 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
conda doc Documentation non-breaking Non-breaking change Python Related to RMM Python API
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants