Skip to content
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

Remove import workarounds #190

Merged
merged 1 commit into from
Dec 9, 2024
Merged

Remove import workarounds #190

merged 1 commit into from
Dec 9, 2024

Conversation

visr
Copy link
Member

@visr visr commented Nov 26, 2024

Fixes #124.
This only removes lines because the imports didn't seem to be used.

@visr visr assigned rbruijnshkv and unassigned rbruijnshkv Nov 26, 2024
@visr visr requested a review from rbruijnshkv November 26, 2024 12:27
Copy link
Collaborator

@rbruijnshkv rbruijnshkv left a comment

Choose a reason for hiding this comment

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

As there is still a slight offset in node_id's when comparing the outcome of the AGV model at our HKV server and GitHub, I can not merge this PR as the information in the feedback forms will be lost. My code needs this original code to keep up with the development of the LHM. Can only merge this when when the cause of change in node_id is found and solved.

@visr
Copy link
Member Author

visr commented Dec 9, 2024

@rbruijnshkv that comment seems to be for #178. Can this go in in the meanwhile?

@visr visr merged commit 932a39b into main Dec 9, 2024
4 checks passed
@visr visr deleted the remove-import-workarounds branch December 9, 2024 12:16
@gijsber
Copy link

gijsber commented Dec 10, 2024

@rbruijnshkv @visr About that mismatch in Ids.
What about creating a mapping table with the ids in model 1 and 2 (based on coordinates match) and use that to update the feedback form to use the new ids.

@rbruijnshkv
Copy link
Collaborator

Good suggestion. I do see some bottlenecks in this though: sometimes there are two nodes stacked on eachother on the exact same location due to the delivered data (i.e. two pumps on the same location, two weirs on the same location, a combination of a weir/pump on the same location, etc). This can't be tackled by a mapping table based on the location.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Remove workarounds to import packages
3 participants