-
Notifications
You must be signed in to change notification settings - Fork 0
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
121 transformatie naar hydamo #131
base: main
Are you sure you want to change the base?
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi Stijn,
het ziet er profi uit. Ik kan er inhoudelijk niets veel op aanmerken.
Heb je of kun je nog gebruik maken van dit: https://github.com/HetWaterschapshuis/HyDAMOValidatieModule/blob/ff9df5291b1f5522ac5b0e62260601c9acba64a1/hydamo_validation/datamodel.py#L295
Je weet dat we DAMO versie 2.4 draaien bij HHNK? Ik had begrepen dat er nog geen 2.4 is van HyDAMO, ik heb er wel om gevraagd bij HWH, maar nog niets van gezien. Het verschil zal we niet zo groot zijn, zeker niet voor HydroObject.
Ik mis nog iets van een test of een voorbeeld hoe dit te gebruiken. Misschien kun me daar donderdag nog wat bij helpen?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Prima opzet, duidelijk wat er allemaal gebeurt. Een testdataset zou helpen, nog een paar opmerkingen.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Graag zelf de ruff formatting draaien
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Naamgeving map en bestand niet gelijk, 2_3 -> 23
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Geen gelijke naamgeving: 2_3 -> 2.3.
Zie bij de hydamo validatiemodule ook allemaal punten staan in de namen nog (https://github.com/HetWaterschapshuis/HyDAMOValidatieregels/blob/main/validation_rules/ValidationRules_1.3.json) Ben niet direct fan van extra punten in namen.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Originele bron verwijzing?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Wat is dit? Staat dit niet toevallig ergens anders al, mogelijk linkje gebruiken? Of gaan we hier een 'eigen' versie van beheren?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ik heb dit destijds, anderhalf jaar geleden, ontvangen van via de mail van het waterschapshuis. We zouden even moeten onderzoeken of dit inmiddels online ergens aangeboden wordt, dan kunnen we ook 2.4 gebruiken ipv 2.3
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We hebben vaste bestanden/resources die geen code zijn in https://github.com/threedi/hhnk-threedi-tools/tree/main/hhnk_threedi_tools/resources staan. Wellicht dat je daar in de readme er dan ook bij kan zetten waar het vandaan komt.
|
||
def get_field_type(self, column_name, layer_name): | ||
""" | ||
Retrieves the field type of a specific attribute in a definition. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ruff gaat hier veel gele lijntjes neerzetten om; https://docs.astral.sh/ruff/rules/non-imperative-mood/
|
||
return column | ||
|
||
def convert_field_type(self, column, field_type): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Werkt column.astype(float)
en zelfde voor de andere dtypes niet ook gewoon? Misschien dat er dan nog mapping nodig is van 'string' -> 'str', maar dat kan ook in 1 dict.
Dan lijkt deze functie me overbodig.
|
||
WATERSCHAPSCODE = 12 # Hoogheemraadschap Hollands Noorderkwartier | ||
|
||
class Converter: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Misschien een iets beschrijvendere naam.
|
||
def __init__(self, DAMO_path, HyDAMO_path, hydamo_schema_path, damo_schema_path, layers): | ||
self.DAMO_path = Path(DAMO_path) | ||
self.HyDAMO_path = Path(HyDAMO_path) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Validatie / controle of output/lagen al bestaat? Deze anders verwijderen?
layer_gdf[column_name] = self.convert_column(layer_gdf[column_name], column_name, layer_name) | ||
return layer_gdf | ||
|
||
def convert_column(self, column, column_name, layer_name): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Merk met doornemen dat ie nu wel 5 functies diep lijkt te gaan. Maakt het wat moeilijker te lezen. Weet nog niet helemaal wat handig is, of meer code uitschrijven in de aanroepende functies zodat je dit niet nodig hebt, of misschien deze helper functies als private zetten? def _convert_column(...
No description provided.