Fix test failing for environments where floating point numbers are localized #12667
Conversation
This is to avoid a bug in resdata where ascii irap is erronously applied locale formatted floats. Note that there is no longer a need to transpose the output from load_parameters as that was only there because resdata.Surface::__setitem__ would use F-order of indices.
eivindjahren
changed the title
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #12667 +/- ##
==========================================
- Coverage 90.67% 90.64% -0.04%
==========================================
Files 429 429
Lines 29803 29803
==========================================
- Hits 27025 27015 -10
- Misses 2778 2788 +10
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
achaikou
left a comment
achaikou
left a comment
There was a problem hiding this comment.
😻 Looks like qt no longer bothers my tests and the problem is fixed. Thank you!
From my side, just a couple of questions about the established approach to the code 🙂
| config += config_str | ||
| expect_surface = Surface( | ||
| nx=2, ny=2, xinc=1, yinc=1, xstart=1, ystart=1, angle=0 | ||
| expect_surface = RegularSurface( |
There was a problem hiding this comment.
Would appreciate some grand knowledge
So there are surfaces in 3 libraries:
resdata.Geometry.Surface(used before for writing)xtgeo.RegularSurface(used now for writing)surfio.IrapSurface(used now for reading surface data in create_runpath - ErtConfig - EnsembleConfig - SurfaceConfig)
What is the difference between those libraries and when should each be used?
What is the reason for read-write mismatch?
| actual_surface -= expect_surface | ||
| assert (actual_surface <= 1e-5).all() |
There was a problem hiding this comment.
This looks somewhat weird to me.
Is this an established ert-way to compare floating points regardless of values even when we deal with two surfaces with well defined integer values?
I would have expected something more straighforward, like
assert np.array_equal(actual_surface.values, expect_surface.values)
| xori=1.0, | ||
| yori=1, | ||
| rotation=0.0, | ||
| values=values, |
There was a problem hiding this comment.
Intellisense/pylance or something keeps screaming at me that there is a type mismatch (values are list of lists, when xtgeo requires just a list)
Another place in the test it also complains about who-knows-what.
So my question is: how strict should we be to such warnings? I see those pop here and there.
Given all that "mypy" type checking I thought we cared?
3 participants
This avoids an issue with resdata.geometry.Surface where floating point formatting
is erronously localized in irap files. This only applies to one test in ert so it is enough
to work around it by using xtgeo instead.
git rebase -i main --exec 'just rapid-tests')When applicable