Tiles

[tmillenaar]

No description provided.

[codecov]

Codecov Report

Attention: Patch coverage is 99.11765% with 3 lines in your changes missing coverage. Please review.

Project coverage is 95.81%. Comparing base (65cf067) to head (0430245).

Files	Patch %	Lines
gridkit/index.py	50.00%	2 Missing ⚠️
gridkit/rect_grid.py	96.00%	1 Missing ⚠️

Additional details and impacted files

@@                Coverage Diff                @@
##           staging_1.0.0      #92      +/-   ##
=================================================
+ Coverage          95.50%   95.81%   +0.30%     
=================================================
  Files                 42       46       +4     
  Lines               4097     4419     +322     
=================================================
+ Hits                3913     4234     +321     
- Misses               184      185       +1     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[tmillenaar]

I encountered an issue related to mutability.
The Python Tile object has a reference to the python grid and to the PyO3 tile. The PyO3 tile then has a reference to the PyO3 grid. This results in the scenario where when the python grid is updated, the grid that is associated with the tile is also updated, but the grid that is associated with the PyO3 tile is not updated.
Since this may sound confusing, consider the following case:

Using commit: 8f19e69

>>> from gridkit import HexGrid
>>> grid = HexGrid(size=1)
>>> from gridkit import HexGrid, Tile
>>> tile = Tile(grid, [0,0], 5, 5)
>>> tile.corners()
Rot: 0
array([[0.        , 4.33012702],
       [5.        , 4.33012702],
       [5.        , 0.        ],
       [0.        , 0.        ]])
>>> from gridkit import RectGrid, Tile
>>> grid = RectGrid(size=1)
>>> tile = Tile(grid, [0,0], 5, 5)
>>> tile.corners()
Rot: 0
array([[0., 5.],
       [5., 5.],
       [5., 0.],
       [0., 0.]])
>>> tile.grid.rotation = 10
>>> tile.grid.rotation
10
>>> tile.corners()
Rot: 0
array([[0., 5.],
       [5., 5.],
       [5., 0.],
       [0., 0.]])

Notice here how the rotation is updated, but the corners as returned by tile.corners() do not reflect this change. The tile.grid (python grid) needs to somehow be coupled to the tile._tile.grid (PyO3 grid).
The PyO3 grid tile._tile.grid is in essence a copy of tile.grid._grid. But these diverge as soon as one of them is updated. In this example it is tile.grid._grid that gets updated and tile._tile.grid that stays the same.

The desired behavior is for the python grid to be mutable in a way where modifying the grid is propagated to all the tiles. This makes updating all the tiles cheap. The user can then opt-in to immutability by passing a copy of the grid to the tile on initiation. But to allow the mutability, the setup needs rethinking.

[tmillenaar]

A robust/safe way to fix this problem is to make Tile._tile a property, where a new PyO3 tile class is instantiated every time, but this means that every call to an attribute or method on the python Tile object is then creating a fresh PyO3 class. This feels unnecessarily inefficient.

[tmillenaar]

Fundamentally, the PyO3 objects are not mutable.
In the ideal case the PyO3 objects are updated whenever the overarching python grid is updated, but keeping track of this state becomes very cumbersome, hard to reason about and error prone. There are therefore two feasible options that I can see:

1. implement the full Tile implementation in python only, not in rust
2. make the grid reference immutable

This is where, in order to make an informed decision I have to consider the end goal. What I consider to be the end goal of the Tile class is to be used in a DataTile that will replace the BoundedGrid class. The reasoning is that the tiles can deal with rotated grids where the BoundedGrid implementation can not. The grid associated with the DataTile will be immutable because the data only makes sense with that specific grid configuration. I was hoping to enforce this immutability on the DataTile level and not on the Tile level, but if we want to use PyO3 objects I think we are forced to also make the Tile immutable. If the Tile is implemented in pure Python for the sake of mutability, we cannot then create the DataTile, which builds on top of Tile, in Rust anymore. Since the DataTile will have multiple methods related to data processing such as interpolation, I don't think it is wise to force these methods to be in Python only for the sake of mutability. Hence I believe it best to keep the Tile implementation mostly in Rust and have a thin Python wrapper that will have to make a copy of the grid in order to avoid Python grid associated with the Tile from diverging with the PyO3 grid associated with the PyO3 tile.