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

Added node inflow report into nodes dataframe #70

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

mehmetbercan
Copy link

No description provided.

@aerispaha
Copy link
Member

Thanks for this contribution, @mehmetbercan! The reason the checks are failing is a because the new column headers introduced by the "Node Inflow Summary" section of the RPT file conflict with column headers already brought into the nodes dataframe by default. This results in this error a failed test_model_to_networkx unit test. Here's the main error message:

ValueError: columns overlap but no suffix specified: Index(['MaxLatInflow', 'MaxTotalInflow', 'LatInflowV', 'TotalInflowV', 'FlowBalErrorPerc']

I think we need to consider a couple things to resolve this:

  1. we should be consistent with what columns are called
  2. we should potentially let users define what columns should be return when calling model.nodes(), similar to Optionally specify which attributes to return in model.conduits() ? #36
  3. an easy fix would be to specific a left/right suffix to Pandas when attempting to join dataframes with non-unique columns names

@aerispaha aerispaha added this to the v0.4.0 milestone Apr 22, 2020
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.

2 participants