grass.temporal: fix regression in printing metadata#4328
grass.temporal: fix regression in printing metadata#4328veroandreo merged 2 commits intoOSGeo:mainfrom
Conversation
ninsbl
left a comment
There was a problem hiding this comment.
Looks good in general.
The one comment is probably a leftover and can be removed?
You may consider using just one print statement and multile f-strings in the print function. That would be minimally faster and less code, but that is not essential, I guess...
Co-authored-by: Stefan Blumentrath <[email protected]>
Right, but probably out of scope of this PR, there are many places in there that would have to be changed. |
veroandreo
left a comment
There was a problem hiding this comment.
Works as expected now! Thanks!
|
Before merging, can we rerun with main merged here, as tests on Windows are more strict now |
Seems I clicked merge while you were typing this? ops... It was all green ;-) |
|
Ya, once I saw the email I looked at it and was writing. It was all green from 60 commits behind main. We'll see how it goes, maybe it'll need a follow-up or not. We'll have our answer in an hour or so |
|
So yes, there are some unexpected successes, as some new tests are passing. If the affected tests were solved by this PR (not an unintended side-effect that we would want to be signalled), we need to remove the xfail_windows decorator on these tests. |
The choice is between requiring the updated branch before merging (resulting in more CI wait time) or need to judge case-by-case if update is needed with occasional failed runs on main (current state). |
|
Can I backport this or anything needed first in G84? |
* grass.temporal: fix regression in printing metadata * Update python/grass/temporal/metadata.py Co-authored-by: Stefan Blumentrath <[email protected]> --------- Co-authored-by: Stefan Blumentrath <[email protected]>
Fixes #4182 (t.info fails for single registered raster). I tried to work with the current design, although it might have been just easier to revert 7df1a7e, the method inheritance doesn't work very well there.
Besides the reported error,
STDSMetadataBase.print_info()was also failing. I tested it with the currently deactivated doctests (there are issue running the doctests unrelated to this).