Replies: 12 comments 19 replies
-
Generally love how these look, it's really tight, informative and covers every case I can think of. Most minor of suggestions - could label the points and bounds consistently with the other attributes (e.g. |
Beta Was this translation helpful? Give feedback.
-
While I was at the previous, I also thought to replace "<lazy>" with actual dask array printouts (in the long-form).
How does that seem? |
Beta Was this translation helpful? Give feedback.
-
Just for note: I'm still making some changes here for specific things... (1.) date-units calendar
In a way, it would be "better" if (2.) lazy arrays (3.) too-long arrays in repr
rather than just
-- i.e. we use b) - in similar cases where a summary with ellipsis won't fit, but a single point can, it will now print (4.) bounds shape
|
Beta Was this translation helpful? Give feedback.
-
Okay I just thought of something else...
So, versions of some of the above Coord printouts, compared with/without column alignment, might look like this ...
Looking at those, I suppose it is more consistent with Cube printouts, Does anyone have strong feelings about this ? |
Beta Was this translation helpful? Give feedback.
-
Looking much better! I've always found coords quite hard to read so this is a big improvement. Below are the comments I noted down as I was reading through the examples (I can split these into multiple posts if you would prefer!) Small co with real points but lazy boundsI'm confused by this one. You say the points are real but then they are displayed as
Is this just a mistake, or intentional? repr inconsistenciesThere seem to be some inconsistencies as to what is included in the repr, in particular whether the points and/or shape are displayed. My interpretation of what going on is that
This may be confusing to a user if they are not familiar with what to expect. Say a user pretty much never handles time coordinates but then one day they have a time coordinate, if they look at the repr they will be confused as to where the points went.
repr tersenessI'm not so sure about using the repr in this way. Python docs for repr say that
And then the attributesIt's nice that things are spaced out, on their own lines, but this may get a bit messed up by attributes. What if the attributes dict was particularly long? We could display them similar to a cube does where they each have their own line
|
Beta Was this translation helpful? Give feedback.
-
@lbdreyer repr terseness
It's entirely designed for use at the command line, where a command result prints as repr.
|
Beta Was this translation helpful? Give feedback.
-
@lbdreyer Small co with real points but lazy boundsThis was a under previous implementation, which I think was bugged. |
Beta Was this translation helpful? Give feedback.
-
@lbdreyer repr inconsistencies
Again, I have improved this somewhat, since the original examples.
(N.B. these first 2 are entirely down to numpy, and can be adjusted with printoptions, e.g. precision + max_width).
The choice between style is (and always was) entirely determined by the printing width : I simply output the longest form that will fit. Meanwhile, the shape is currently only shown when we didn't print all the values. Which was mainly to make the scalar coords easier to look at. |
Beta Was this translation helpful? Give feedback.
-
Unfortunately, it looks like printing examples was basically a bad idea, since the originals are no longer up-to-date, but changing them will confuse the recorded comments + discussion ! I think my best approach is probably to include the latest versions instead.
Note: I'm busy writing real tests, including hopefully all cases above, so this should make it easier to track any subsequent changes. |
Beta Was this translation helpful? Give feedback.
-
@lbdreyer attributesI think this is a good point.
|
Beta Was this translation helpful? Give feedback.
-
Whilst reviewing the PR I started having second thoughts about the repr. I'm concerned we're demoting bounds to second class citizens by not including them in the repr.
One way to avoid this is to just not include any values in the repr at all. Perhaps instead just include the shape of the points and the bounds for coords, and data for cell measures and ancil vars. |
Beta Was this translation helpful? Give feedback.
-
Still thinking about fixing the attributes printout to have each one on its own line The Cube method is currently avoiding the use of colons as separators, but looking at it again I'm not sure it is doing such a great job. For instance ...
What I'm seeing there is that the alignment is not that great, and it needs to quote string values, to avoid confusing the value with the key (!)
... however, I still think the colons aren't that valuable, it probably looks OK without them, as long as there is a minimum of two spaces ...
|
Beta Was this translation helpful? Give feedback.
-
Please see #4499 for initial code (hacky but you can test it) and sample outputs.
This new format actually applies (or will do) to all _DimensionalMetadata,
So it should also replace the special-case code currently provided in MeshCoord.
With this in hand, we can then easily make a nicer printout for Mesh, too.
Beta Was this translation helpful? Give feedback.
All reactions