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

Added transpose stub to match upcoming capability in OOPS #128

Merged
merged 2 commits into from
Jan 20, 2025

Conversation

DanStoneMO
Copy link
Contributor

@DanStoneMO DanStoneMO commented Jan 16, 2025

Description

Adds a stub for the transpose method to State.h in order to comply with the method's upcoming addition to OOPS.

Issue(s) addressed

See discussion: https://github.com/orgs/JCSDA-internal/discussions/164

Dependencies

This should be merged alongside the equivalent PRs for lfric-jedi and ops-um-jedi, but is not dependent on them.

Impact

Expected impact on downstream repositories or workflows:
None unless transpose functionality is to be added to orca-jedi, in which case the stub will need to be expanded
into a full method.

Checklist

  • [n/a] I have updated the unit tests to cover the change
  • [n/a] New functions are documented briefly via Doxygen comments in the code
  • [n/a] I have linted my code using cpplint
  • [✓] I have run the unit tests
  • [✓] I have run mo-bundle to check integration with the rest of JEDI and run the unit tests under all environments
    See mo-bundle test output:
    http://fcm1/cylc-review/taskjobs/dstone/?suite=mob_transpose

@DanStoneMO DanStoneMO requested a review from twsearle as a code owner January 16, 2025 13:56
@DanStoneMO DanStoneMO self-assigned this Jan 16, 2025
Copy link
Contributor

@ss421 ss421 left a comment

Choose a reason for hiding this comment

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

Thanks Dan :-)

Updated NotImplemented error message to match repository style

Co-authored-by: Toby Searle <[email protected]>
Copy link
Collaborator

@twsearle twsearle left a comment

Choose a reason for hiding this comment

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

Thanks very much for doing this!

@DanStoneMO
Copy link
Contributor Author

@twsearle this might not need the coordinate merge tag, since there's no mechanical dependencies on the other transpose-stub PRs

@twsearle
Copy link
Collaborator

@twsearle this might not need the coordinate merge tag, since there's no mechanical dependencies on the other transpose-stub PRs

ahh sorry I thought this needed the OOPS PR to be merged before it could be merged. Appears not will merge now!

@twsearle twsearle merged commit 543db66 into develop Jan 20, 2025
2 checks passed
@DanStoneMO DanStoneMO deleted the feature/transpose-stub branch January 21, 2025 16:35
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