Skip to content

Meshcoord printout [AVD-1592]#4054

Merged
bjlittle merged 3 commits intoSciTools:mesh-data-modelfrom
pp-mo:meshcoord_printout
Mar 9, 2021
Merged

Meshcoord printout [AVD-1592]#4054
bjlittle merged 3 commits intoSciTools:mesh-data-modelfrom
pp-mo:meshcoord_printout

Conversation

@pp-mo
Copy link
Member

@pp-mo pp-mo commented Mar 5, 2021

🚀 Pull Request

Description

Added string representations for MeshCoord

Waiting until #4053 is in : Which this is based on.
(so it won't merge into the target branch, as yet)

I have made the str/repr versions slightly different, following discussion with @bjlittle
I'm still a little uncertain that the results are the most useful, though:

  • if you enter a MeshCoord at the command line, you get "repr", which shows a mesh as <Mesh object at 0x{address}>,
    • whereas the "str" form attempts a more human-readable account of the Mesh, as Mesh('name')
  • This corresponds with the "standard" Python usage ...
  • ... but, for a cube, the "repr" version is the short form, whereas print(cube) gives full details.
    • .. so making repr the "more precise and less human" is against what we do for cubes.

Consult Iris pull request check list

@pp-mo pp-mo changed the base branch from master to mesh-data-model March 5, 2021 19:56
@pp-mo pp-mo marked this pull request as draft March 5, 2021 19:57
@pp-mo pp-mo changed the title Meshcoord printout Meshcoord printout [AVD-1592] Mar 8, 2021
@bjlittle bjlittle self-assigned this Mar 8, 2021
@bjlittle
Copy link
Member

bjlittle commented Mar 8, 2021

* .. so making repr the "more precise and less human" is **_against_** what we do for cubes.

@pp-mo Point taken.

To be honest we should rationalise and align all our __repr__ and __str__ usage throughout iris... it's well overdue and the state of affairs that we find ouselves in is a fair reflection on the unopinionated, organic way in which iris has developed over the years

Copy link
Member

@bjlittle bjlittle left a comment

Choose a reason for hiding this comment

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

@pp-mo Some suggested changes... nothing major. See what you think.

Otherwise LGTM 👍

@pp-mo
Copy link
Member Author

pp-mo commented Mar 9, 2021

Rebased onto #4053 (effectively).
But, this may need doing again.

In the meantime, attending to the existing comments above (though now outdated).

@pp-mo pp-mo marked this pull request as ready for review March 9, 2021 15:34
@pp-mo
Copy link
Member Author

pp-mo commented Mar 9, 2021

I think that's it for the outstanding review issues.
I'm taking this out of Draft, but it still won't look nice here (or merge into the target) until #4053 is in.

@pp-mo pp-mo marked this pull request as draft March 9, 2021 15:43
@pp-mo pp-mo force-pushed the meshcoord_printout branch from 62e4399 to 1568a8a Compare March 9, 2021 16:58
@pp-mo pp-mo marked this pull request as ready for review March 9, 2021 16:59
@pp-mo
Copy link
Member Author

pp-mo commented Mar 9, 2021

Rebased again ( 😓 )
Ready to go @bjlittle !

@pp-mo pp-mo force-pushed the meshcoord_printout branch from eb44556 to 6d2b121 Compare March 9, 2021 18:37
@bjlittle bjlittle merged commit 3b8f492 into SciTools:mesh-data-model Mar 9, 2021
@pp-mo pp-mo deleted the meshcoord_printout branch January 21, 2022 12:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants