Skip to content

Conversation

@mcflugen
Copy link
Member

If a variable does not have a grid, then its location should be reported as "none". This means that get_var_grid for that variable will not be implemented or, if it is, the return value will be meaningless.

@mcflugen mcflugen requested review from Copilot and mdpiper May 31, 2025 23:19
Copy link

Copilot AI left a 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 updates the project documentation by removing outdated ReStructuredText files and replacing them with Markdown versions, along with several ancillary documentation changes such as updating dependency lists and reformatting contributor and citation files.

  • Migrated BMI documentation from RST to Markdown (getter_setter, control_funcs, best_practices, etc.).
  • Removed legacy templates (sidebarintro, localtoc, links) and updated documentation build requirements and workflows.
  • Revised ancillary files including CONTRIBUTING, CODE-OF-CONDUCT, CITATION, and AUTHORS to use Markdown syntax.

Reviewed Changes

Copilot reviewed 61 out of 61 changed files in this pull request and generated no comments.

Show a summary per file
File Description
docs/source/bmi.getter_setter.* Replaced RST version with a new Markdown version
docs/source/bmi.control_funcs.* Migrated control functions documentation to Markdown
docs/source/bmi.best_practices.* Converted best practices documentation to Markdown
Various template files Removed outdated HTML sidebar and TOC files
docs/requirements.txt Updated documentation dependencies and added a new BMI map dependency
README.rst, CITATION.rst, AUTHORS.rst Removed RST files in favor of new Markdown counterparts
CONTRIBUTING.md, CODE-OF-CONDUCT.md Updated formatting and link syntax for clearer guidance
.github/workflows/docs.yml Removed workflow file; verify that the docs build is maintained via alternate automation
Comments suppressed due to low confidence (3)

README.rst:1

  • The removal of README.rst without a clear replacement might leave the project without an introductory document. Please ensure there is a new README file (preferably in Markdown) to guide new users.
Entire README.rst removed

.github/workflows/docs.yml:1

  • The deletion of the documentation build workflow may disrupt automated documentation builds. Verify that an alternative process is in place to build and deploy the docs.
Entire docs.yml workflow file removed

docs/source/bmi.getter_setter.md:55

  • Ensure that the cross-reference syntax (e.g., {ref}get-var-type) is properly resolved by the new Markdown parser (myst-parser) to maintain link integrity in the final output.
and can be determined through {ref}`get-var-type`, {ref}`get-var-nbytes`, etc.

@mcflugen mcflugen changed the base branch from develop to stable May 31, 2025 23:20
@mcflugen mcflugen requested a review from Copilot May 31, 2025 23:20
Copy link

Copilot AI left a 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 updates the BMI variable functions documentation to include "none" as a valid return value for get_var_location when a variable is not attached to a grid.

  • Added "none" to the list of valid variable locations.
  • Clarified that get_var_grid will not be implemented for variables with a "none" location.

@mcflugen
Copy link
Member Author

mcflugen commented Jun 2, 2025

@PhilMiller, @mdpiper I've added a note to alert users that the grid location of "none" is not formally part of the current version of BMI but that it may be added to future versions. Do you think this is a good idea? Maybe different wording and give a link to a (not yet existing) github issue where this is being discussed?

@PhilMiller
Copy link
Member

Could we get a coherent treatment of scalar and none in a single proposal, rather than split between this and #184? For instance, why can't we just elaborate the specification of scalar to say that unlike node/face/edge it does not imply that get_var_grid will answer for it, rather than replacing scalar with none?

@mcflugen
Copy link
Member Author

mcflugen commented Jun 2, 2025

Yes, we can do that. I've created #185 for this.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants