Couple riCouple river junction node to nearest river locations. This tool can reduce a lot of manual configuration time if client has hundred and thousand river junction needed to be configured.#112
Conversation
…reduce a lot of mannual configuration time if client has hundrad and thousand river junction needed to be configured.
|
Hi @ryan-kipawa, I made a tool. This tool is to couple river junction node to nearest river location. The purpose here is to reduce the manual configuration work for Modelers. Can you please review it? There is a build error. I don't know how to fix it. I haven't add clr reference in the init.py file. I don't want mikepluspy to load it at the first place. |
|
I'll take a look. Which versions of MIKE+ is the tool relevant for? |
ryan-kipawa
left a comment
ryan-kipawa
left a comment
There was a problem hiding this comment.
I had always considers tools to be a sub-package that exposes tools available in MIKE+. This appears to be a tool that would only be available in MIKE+Py. Do you have more context on the long term vision of this tool?
|
|
||
| clr.AddReference("ThinkGeo.Core") | ||
|
|
||
| from ThinkGeo.Core import BaseShape # noqa: E402 |
There was a problem hiding this comment.
It is our static type checker (mypy) that fails in the build system. It doesn't recognize dynamically created modules. You'll need to make mypy ignore the pattern ThinkGeo.* similar to other pythonnet imports:
Line 72 in 27a36b0
There was a problem hiding this comment.
@ryan-kipawa fixed. And the utility class has been added.
It can work in the previous release version. |
|
@ryan-kipawa get_nearest_river_at method has been added. I will move the couple_river_junction_tool to notebook after you decide where to put the BeginTransaction and EndTransaction. |
ryan-kipawa
left a comment
ryan-kipawa
left a comment
There was a problem hiding this comment.
@ryan-kipawa get_nearest_river_at method has been added. I will move the couple_river_junction_tool to notebook after you decide where to put the BeginTransaction and EndTransaction.
I think it makes sense to add begin_transaction() and end_transaction() to the Database class here:
|
@ryan-kipawa I have updated the script for this PR. Can you review it again? |
| ) | ||
|
|
||
| def begin_transaction(self): | ||
| """Begin the data transaction.""" |
There was a problem hiding this comment.
Add an example to the docstring to provide context on how this would be used
|
|
||
| def begin_transaction(self): | ||
| """Begin the data transaction.""" | ||
| if not self._is_open: |
There was a problem hiding this comment.
I think this should raise an error instead of silently returning
| commit : bool | ||
| true is to commit the data into database, false is to cancel the commit. | ||
| """ | ||
| if not self._is_open: |
There was a problem hiding this comment.
What happens in the case where a user opens the database, starts the database, and then calls close() without ending the transaction?
I think a sensible default is to auto-commit (i.e. assume transactions will always be applied when calling end_transaction or close, unless the user explicitly wants to discard the changes). It is probably most common that a user will want to commit changes.
There was a problem hiding this comment.
@ryan-kipawa The database will be locked when the "begin transaction" happens. So "begin transaction" should always go with "end transaction".
|
|
||
| self._data_table_container.BeginTransaction() | ||
|
|
||
| def end_transaction(self, commit: bool): |
There was a problem hiding this comment.
Consider default to true (see comment below)
There was a problem hiding this comment.
There is an error in this notebook - it should run cleanly
| from DHI.Amelia.Infrastructure.Interface.UtilityHelper import GeoAPIHelper # noqa: E402 | ||
|
|
||
|
|
||
| class SpatialAnalysisUtil: |
There was a problem hiding this comment.
Since the class only holds static methods, they can simply exist in the module itself
|
@ryan-kipawa Thank you for all the comments. I have uploaded another version. Please check. |
2 participants
…reduce a lot of manual configuration time if client has hundred and thousand river junction needed to be configured.