equinor / xtgeo

XTGeo Python class library for subsurface Surfaces, Cubes, Wells, Grids, Points, etc
https://xtgeo.readthedocs.io/en/latest/
GNU Lesser General Public License v3.0
110 stars 56 forks source link

DEP: Deprecate dimension_only argument in `grid_from_roxar()` #1107

Closed tnatt closed 8 months ago

tnatt commented 8 months ago

The dimensions_only argument available in xtgeo.grid_from_roxar() does not work, and this PR resolves #1042.

Its intended use of this argument was to create a grid with correct dimensions that could be used as a template for a new GridProperty. By only extracting the dimensions of a grid, the users could get a performance increase.

Extracting dimensions from a grid is pretty straightforward with the roxar API, hence this bug will not be fixed.

This PR adds a deprecation warning to the dimensions_only argument, and removes all code related to this argument since dimensions_only=True did not work anyway.

tnatt commented 8 months ago

Do you also need to remove the argument from Grid().from_roxar()?

https://github.com/equinor/xtgeo/blob/4716552b103e93b21a8272f35f833c41d304ad5d/src/xtgeo/grid3d/grid.py#L1124-L1126

You might also consider adding this to the migration guide, even if the impact is small.

Nice catch, did not notice that one .. it will be good when we can turn on more pylint options trough ruff to pick up on these 👍 I will add it to the migration guide!

codecov-commenter commented 8 months ago

Codecov Report

Attention: 5 lines in your changes are missing coverage. Please review.

Comparison is base (7e1c87e) 80.02% compared to head (96ada09) 80.00%.

Files Patch % Lines
src/xtgeo/grid3d/grid.py 16.66% 4 Missing and 1 partial :warning:
Additional details and impacted files ```diff @@ Coverage Diff @@ ## main #1107 +/- ## ========================================== - Coverage 80.02% 80.00% -0.03% ========================================== Files 95 95 Lines 13430 13434 +4 Branches 2183 2185 +2 ========================================== Hits 10748 10748 - Misses 1956 1959 +3 - Partials 726 727 +1 ```

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.