-
Notifications
You must be signed in to change notification settings - Fork 79
Add ReadTheDocs configuration for random_landmarking documentation #174
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
Conversation
Pull Request Test Coverage Report for Build 21372281460Details
💛 - Coveralls |
Co-authored-by: MattScicluna <19255250+MattScicluna@users.noreply.github.com>
Co-authored-by: MattScicluna <19255250+MattScicluna@users.noreply.github.com>
MattScicluna
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
please ensure this will include changes in V2 of phate (the random_landmarking)
| # Read the Docs configuration file | ||
| # See https://docs.readthedocs.io/en/stable/config-file/v2.html for details | ||
|
|
||
| version: 2 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
is this version 2 of phate? https://github.com/KrishnaswamyLab/PHATE/releases/tag/2.0.0
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, this is building version 2.0.0. The version: 2 on line 4 refers to the ReadTheDocs configuration file format version (not PHATE version). The actual PHATE version being documented is determined by what's in the code - Python/phate/version.py shows __version__ = "2.0.0". The built documentation correctly shows "PHATE 2.0.0" and includes the random_landmarking parameter.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
This PR adds ReadTheDocs configuration to ensure the PHATE documentation builds successfully and includes all documented parameters, specifically addressing the visibility of the random_landmarking parameter.
Changes:
- Added
.readthedocs.ymlconfiguration file with Python 3.11 build environment and proper installation order - Fixed Sphinx warning by changing
language = Nonetolanguage = "en"inconf.py - Removed circular dependency in
requirements.txtby deleting./Pythonline
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
.readthedocs.yml |
New ReadTheDocs v2 configuration specifying build environment, Sphinx location, and installation order |
Python/doc/source/conf.py |
Fixed deprecated language = None to language = "en" |
Python/doc/source/requirements.txt |
Removed ./Python line that caused circular dependency with ReadTheDocs build |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Yes, the changes are already in V2. The current code is version 2.0.0 (see |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
Copilot reviewed 3 out of 3 changed files in this pull request and generated no new comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
The
random_landmarkingparameter is documented in the Python source but not visible on ReadTheDocs. The parameter exists inPHATE.__init__()and controls whether landmarks are selected randomly vs. via spectral clustering.Changes
Added
.readthedocs.yml: Explicit build configuration ensures consistent documentation generation across ReadTheDocs environmentsFixed
conf.py: Changedlanguage = None→language = "en"to resolve Sphinx warningFixed
requirements.txt: Removed./Pythonline that created circular dependency with ReadTheDocs pip installThe autodoc configuration in
api.rstalready extracts parameter documentation from docstrings. These changes ensure ReadTheDocs builds successfully and includes all documented parameters.Warning
Firewall rules blocked me from connecting to one or more addresses (expand for details)
I tried to connect to the following addresses, but was blocked by firewall rules:
phate.readthedocs.io/home/REDACTED/work/_temp/ghcca-node/node/bin/node /home/REDACTED/work/_temp/ghcca-node/node/bin/node --enable-source-maps /home/REDACTED/work/_temp/copilot-developer-action-main/dist/index.js(dns block)If you need me to access, download, or install something from one of these locations, you can either:
Original prompt
✨ Let Copilot coding agent set things up for you — coding agent works faster and does higher quality work when set up for your repo.