Skip to content

[feature/capgen-v1-cleanup-01] Add correct comment to supported variable conversions (#767) + bug fixes + unit test updates#768

Open
climbfuji wants to merge 3 commits into
feature/capgen-v1-cleanup-01from
pr-767
Open

[feature/capgen-v1-cleanup-01] Add correct comment to supported variable conversions (#767) + bug fixes + unit test updates#768
climbfuji wants to merge 3 commits into
feature/capgen-v1-cleanup-01from
pr-767

Conversation

@climbfuji

@climbfuji climbfuji commented Jul 13, 2026

Copy link
Copy Markdown
Collaborator

This is PR #767 (thanks @dustinswales for this contribution) with a few bug fixes and unit test updates.

It now targets the branch feature/capgen-v1-cleanup-01 instead of feature/capgen-v1.

I ran the doctests, unit tests, and end-to-end tests locally - they all passed.

@climbfuji climbfuji self-assigned this Jul 13, 2026
@climbfuji climbfuji marked this pull request as ready for review July 13, 2026 18:02
@climbfuji climbfuji requested review from a team as code owners July 13, 2026 18:02
@climbfuji climbfuji changed the title Add correct comment to supported variable conversions (#767) + bug fixes + unit test updates [feature/capgen-v1-cleanup-01] Add correct comment to supported variable conversions (#767) + bug fixes + unit test updates Jul 13, 2026
"""The trailing inline comment must list every active transform, but
must suppress "unit conversion" when the rendered formula is the
identity (formula ``'{var}'`` for dimensionally-equivalent units).
"""The trailing inline comment must list every active transform with the

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

I'll clean those comments up when I get to this file in my review of the entire code base (I just finished phase 1, subdirectory metadata/parse_tools)

self.assertEqual(_transform_comment(a, reverse=True),
'! unit conversion: km to m')

def test_unit_conversion_never_reports_kinds(self):

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Same here. I am not sure that we need unit tests for comments in auto-generated code.

@climbfuji

Copy link
Copy Markdown
Collaborator Author

@dustinswales Can you review and approve/request changes when you get a chance, please?

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