Feature/new api for model database #89

ryan-kipawa · 2025-03-20T11:01:56Z

A new fluent-like API is introduced in this PR for working with the MIKE+ database.

Main changes:

DataTableAccess will be replaced by Database. It's a cleaner implementation with more thorough testing. DataTableAccess would become legacy, but kept around for a while since others are using it.
Table classes are autogenerated for each table in the database, which are accessed though a table collection on Database.tables
Each table class has a flexible, fluent-like API with select, insert, update, and delete (with various conditions).
Each table class has a columns attribute for get an enum of possible column values
Conversion logic between .NET pythonnet objects and python has been centralized a bit more

I think this structure will also allow for future extension in an easy way, since it loosely follows the MIKE+ design.

Closes:
#90
#35
#44
#27
#17

Here's a brief comparison of the old vs new API:

Opening database

Old API

data_access = DataTableAccess("model.sqlite")
data_access.open_database()

# Close when done
data_access.close_database()

New API

# Initialize with auto_open=True (the default)
db = Database("model.sqlite")
# Close when done
db.close()

# Or use context manager for automatic open/close
with Database("model.sqlite") as db:
    # Work with database
    pass

Querying Data

Old API

# Get all MUIDs in a table
muids = data_access.get_muid_where("msm_Link")

# Get specific fields for a single record
fields = ["Diameter", "Length", "FromNode"]
values = data_access.get_field_values("msm_Link", "Link_2", fields)

# Get values for all records with a condition
result = data_access.get_muid_field_values("msm_Link", fields, "Diameter > 1.0")

New API

# Access a table through the tables collection
link_table = db.tables.msm_Link

# Get all MUIDs with optional ordering
muids = link_table.get_muids(order_by="Diameter", descending=False)

# Query with chainable methods
result = link_table.select(["Diameter", "Length", "FromNode"]) \
                  .where("Diameter > 1.0") \
                  .order_by("MUID") \
                  .execute()

# Convert to pandas DataFrame
df = link_table.select(["Diameter", "Length"]) \
              .to_pandas()

# Parameterized where statements
min_diameter = 0.5
result = link_table.select() \
                  .where("Diameter > :min_diameter", {"min_dimaeter" : min_diameter}) \
                  .to_pandas()

Inserting Data

Old API

# Insert a new record
values = {
    'Diameter': 2.0, 
    'Description': 'New pipe', 
    'geometry': "LINESTRING (3 4, 10 50, 20 25)"
}
data_access.insert("msm_Link", "new_link_1", values)

New API

# Insert using chainable methods
db.tables.msm_Link.insert({
    "MUID" : "new_link_1",
   "Diameter" : 2.0,
    "Description" : "New pipe",
    "geometry" : "LINESTRING (3 4, 10 50, 20 25)"
})

# Or auto-generate the MUID
db.tables.msm_Link.insert({
    "Diameter" : 2.0,
    "Description" : "New pipe",
    "geometry" : "LINESTRING (3 4, 10 50, 20 25)"
})

Deleting Data

Old API

# Delete a record
data_access.delete("msm_Link", "Link_2")

New API

# Delete using chainable methods
db.tables.msm_Link.delete() \
    .where("MUID = 'Link_2'") \
    .execute()

# Safety feature - to delete all records, must explicitly call all()
db.tables.msm_Link.delete() \
    .all() \
    .execute()

…ideration in future.

…s jinja2

…entations

…ests

…rter

ecomodeller

Looks like a very nice API.

Two comments:

Please make sure that the tests can run
Consider adding mypy to CI to verify that the type hints are correct and stays correct.

ecomodeller · 2025-03-21T12:24:13Z

mikeplus/database.py

+class Database:
+    """Represents a MIKE+ model database."""
+
+    def __init__(self, model_path: str | Path, *, auto_open: bool = True):


When is it useful to not open the database automatically?

Only if you wanted to delay making the connection for some reason. I somehow feel it might be useful for testing or some future use case, but you're right ... I can't think of a strong need for it based on how the code exists now.

ecomodeller · 2025-03-21T12:26:50Z

mikeplus/database.py

+        Returns:
+            List of scenario names
+        """
+        return list(self._scenario_manager.GetScenarios())


This operation (and related) will fail if self._scenario_manager is None, i.e. if the database is not opened.

As reported by mypy.
error: Item "None" of "Any | None" has no attribute "GetScenarios" [union-attr]

Can certainly clean this up. I'd prefer to wait until the overall PR looks okay in design, then go back to fix these up.

Might need to ignore or remove a lot of the type hints related to pythonnet objects. I think this would require stub files for them which could be a lot of work to generate. I like having them cause it facilitates mapping it to C# code. Maybe some practical typing file for these that let us still use them as hints but ignore them?

ryan-kipawa · 2025-03-21T12:54:42Z

Looks like a very nice API.

Two comments:

Please make sure that the tests can run

Consider adding mypy to CI to verify that the type hints are correct and stays correct.

Brief comment on the tests: they do run and pass, just not on CI. This can (partially) be fixed, although still needs to skip licensed tests which have issues I can't resolve in this PR.

ecomodeller · 2025-03-21T13:30:28Z

mikeplus/queries.py

+        if geometry:
+            if isinstance(geometry, str):
+                geometry = DotNetConverter.to_dotnet_geometry(geometry)
+            else:


what happens here? geometry is not None, but it is not being used🤔

I think it's used a few lines below by:

_, inserted_muid = net_table.InsertByCommand( muid, geometry, net_values, )

The geometry stuff hasn't been fully tested enough. There's also a need to differentiate between standard tables and geometry tables, something I hope to incorporate, but maybe as a different PR since this one is quite big already.

ecomodeller · 2025-03-21T13:31:14Z

mikeplus/queries.py

+class UpdateQuery(BaseQuery[List[str]]):
+    """Query class for UPDATE operations."""
+
+    def __init__(self, table: BaseTable, values: dict[str, any]):


Perhaps you meant "typing.Any" instead of "any" (output from mypy)

gedaskir · 2025-03-25T13:52:07Z

@ryan-kipawa Should it always be

from mikeplus.database import Database

or

from mikeplus import Database

could be preferred?

Additionally, would it make sense to have something similar like mikeplus.open(...) method similar to mikeio and mikeio1d?

gedaskir · 2025-03-25T14:03:31Z

mikeplus/database.py

+        if projection_string and srid != -1:
+            raise ValueError("Projection string and SRID cannot be specified together.")
+
+        try:


Does this make the following script https://github.com/DHI/mikepluspy/blob/main/test_utils/database_creators.py. redundant?

gedaskir · 2025-03-25T14:09:07Z

mikeplus/database.py

+    """Represents a MIKE+ model database."""
+
+    def __init__(self, model_path: str | Path, *, auto_open: bool = True):
+        """Initialize a new Database.


Do you think we should settle for Google docstring style instead of NumPy?

gedaskir · 2025-03-25T14:12:42Z

mikeplus/database.py

+            self.open()
+
+    @classmethod
+    def create(


Would it make sense to have an overwrite flag?

ryan-kipawa added 30 commits March 14, 2025 12:41

Add skeleton classes for new model database API

56108db

Add failing test stubs for new model database API

392ca59

Fix import errors of skeleton structure

869f6e5

Add extra pytest fixtures for different scopes (class and module)

84e332f

Add type hints to pytest fixtures

037efb9

Database open implementation and test

3c269b3

Database create implementation and test

42e3712

ModelDatabase tests and implementations for various properties

e3a9af5

Remove active simulation stubs and tests. Simulation API needs recons…

4c03b4b

…ideration in future.

More ModelDatabase tests and implementations for various properties

43efa4f

Expose more read-only properties of DataSource on ModelDatabase

e8940d8

Rename ModelDatabase to Database (model is implied and unambiguous)

95c012a

Implementation of automatic table class generation (progress)

9a057a4

Add developer script for generating tables from root dir

26ad6f5

Refactor auto table generation. Simplified, moved to scripts, and use…

a95ed9d

…s jinja2

Add tests for table generation scripts

94748a8

Add tests for TableCollection

8208c0e

Update tests for tables and queries (progress)

e15088b

Update BaseTable tests and implementations

2fbe9a1

Update BaseQuery tests

e585c69

Add BaseColumns class, and update auto table gen with specific implem…

0ecc205

…entations

SelectQuery tests and implementation (exlcuding where/order clauses)

0aa963c

InsertQuery tests and implementation

b3b95ce

UpdateQuery and DeleteQuery implementatoins and tests (excluding where)

7a8e26a

Make mutating all rows require explicit all() invocation

1109ae9

Minor fix to table generation test

cd0d7dd

SelectQuery where() tests and implementation

8279edb

Update UpdateQuery and DeleteQuery to implement where clause() with t…

6cde7d5

…ests

Prevent double execution. Lift execute() to BaseQuery

e075b2e

Implement order_by for SelectQuery with tests

fe20aeb

ryan-kipawa added 4 commits March 20, 2025 11:57

Pull pythonnet conversion logic out of queries, introduce DotNetConve…

e113e3f

…rter

Fix ruff errors

3639654

Ruff format

c44f3b2

Initial auto generated table classes

ac43af6

ryan-kipawa marked this pull request as ready for review March 20, 2025 13:39

ryan-kipawa requested review from ecomodeller and gedaskir March 20, 2025 13:39

ryan-kipawa added 2 commits March 20, 2025 15:53

Fix minor test issues

6dfd984

Change temporary directory creation to work with 3.9

6b3ba82

ryan-kipawa requested a review from wuwwen March 20, 2025 21:09

ecomodeller reviewed Mar 21, 2025

View reviewed changes

gedaskir reviewed Mar 25, 2025

View reviewed changes

mikeplus/database.py

self.open()

@classmethod

def create(

Copy link

Collaborator

gedaskir Mar 25, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would it make sense to have an overwrite flag?

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Feature/new api for model database #89

Feature/new api for model database #89

ryan-kipawa commented Mar 20, 2025 •

edited

Loading

ecomodeller left a comment

ecomodeller Mar 21, 2025

ryan-kipawa Mar 21, 2025

ecomodeller Mar 21, 2025

ryan-kipawa Mar 21, 2025 •

edited

Loading

ryan-kipawa commented Mar 21, 2025

ecomodeller Mar 21, 2025

ryan-kipawa Mar 21, 2025

ecomodeller Mar 21, 2025

gedaskir commented Mar 25, 2025

gedaskir Mar 25, 2025

gedaskir Mar 25, 2025

gedaskir Mar 25, 2025

Feature/new api for model database #89

Are you sure you want to change the base?

Feature/new api for model database #89

Conversation

ryan-kipawa commented Mar 20, 2025 • edited Loading

Opening database

Old API

New API

Querying Data

Old API

New API

Inserting Data

Old API

New API

Deleting Data

Old API

New API

ecomodeller left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ryan-kipawa Mar 21, 2025 • edited Loading

Choose a reason for hiding this comment

ryan-kipawa commented Mar 21, 2025

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

gedaskir commented Mar 25, 2025

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ryan-kipawa commented Mar 20, 2025 •

edited

Loading

ryan-kipawa Mar 21, 2025 •

edited

Loading