search

USEPA / WNTR

An EPANET compatible python package to simulate and analyze water distribution networks under disaster scenarios.
Other
318 stars 184 forks source link

leaflet map popups #416

Open MunsakaPKM opened 7 months ago

MunsakaPKM commented 7 months ago

Greetings, I'm struggling to understand the add_to_link_popup and add_to_node_popup parameters, and need some guidance. I quoted part of the documentation below where it talks about adding popups. Despite multiple attempts, I'm encountering errors. Could someone please help, I'm particularly looking for an example of the expected DataFrame structure. I've been getting errors, including <class 'pandas.core.frame.DataFrame'> and Exception: 'Series' object has no attribute 'iteritems', which is confusing since I'm not passing a Series. Any clarification would be greatly appreciated

" add_to_node_popup : None or pd.DataFrame, optional To add additional information to the node popup, use a DataFrame with node name as index and attributes as values. Column names will be added to the popup along with each value for a given node.

add_to_link_popup : None or pd.DataFrame, optional
    To add additional information to the link popup, use a DataFrame with 
    link name as index and attributes as values.  Column names will be added
    to the popup along with each value for a given link."
kbonney commented 7 months ago

This error occurs because we are using deprecated API for pandas Series objects as described on this pandas documentation page.

The simple fix is to use the correct method items for a series rather than iteritems. @MunsakaPKM, if you are interested in contributing to WNTR please feel welcome to create a PR to address this.

MunsakaPKM commented 6 months ago

Dear Kirk, Thank you so much for your prompt response and for considering me to contribute to WNTR! As a student, I'm thrilled to have the opportunity to contribute to a project of this importance. I'm eager to learn and gain more experience. However, I must admit that I'm new to open-source development and could benefit from guidance on how to get started. Would it be possible for you to provide some advice on creating a PR ? Any tips or resources you can share would be appreciated. Thanks again for your help and support. Best regards, Prince Munsaka

On Mon, Apr 22, 2024 at 4:13 PM Kirk Bonney @.***> wrote:

This is caused because we are using deprecated API for pandas Series objects as described on this pandas documentation page https://pandas.pydata.org/pandas-docs/version/1.5/reference/api/pandas.Series.iteritems.html .

The simple fix is to use the correct method items for a series rather than iteritems. @MunsakaPKM https://github.com/MunsakaPKM, if you are interested in contributing to WNTR please feel welcome to create a PR to address this.

— Reply to this email directly, view it on GitHub https://github.com/USEPA/WNTR/issues/416#issuecomment-2069602164, or unsubscribe https://github.com/notifications/unsubscribe-auth/BEHWEYXH7F3LTVNNZUMLDXDY6ULJVAVCNFSM6AAAAABGQO23AKVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDANRZGYYDEMJWGQ . You are receiving this because you were mentioned.Message ID: @.***>

kbonney commented 6 months ago

Sure, here are some steps to help get you started:

  1. Create a fork of WNTR and clone it locally.
  2. Install WNTR following the developer instructions , except instead of using USEPA's WNTR, use the WNTR from your fork. I recommend installing to a fresh python environment.
  3. Make the changes we discussed to the code on a new branch. You can test these changes by running pytest wntr, but they will also be automatically tested when you create the pull request.
  4. Commit these changes and push the branch to your fork.
  5. Make a PR for your new branch. Fill out the template that we provide. Once you done that we can look at the next steps.
kbonney commented 6 months ago

Since this bug is not showing up in our current test suite, there should also be an update to the tests to make sure this code is covered properly. I wouldn't worry about this in your PR @MunsakaPKM, I can take care of it on the side.

MunsakaPKM commented 6 months ago

Thank you, I appreciate it. I will review the instructions and begin working on the steps at my earliest convenience. Once I have made progress, I will keep you informed and proceed accordingly.

On Wed, Apr 24, 2024 at 6:01 PM Kirk Bonney @.***> wrote:

Since this bug is not showing up in our current test suite, there should also be an update to the tests to make sure this code is covered properly. I wouldn't worry about this in your PR @MunsakaPKM https://github.com/MunsakaPKM, I can take care of it on the side.

— Reply to this email directly, view it on GitHub https://github.com/USEPA/WNTR/issues/416#issuecomment-2075296922, or unsubscribe https://github.com/notifications/unsubscribe-auth/BEHWEYVFBS7TYJ4LEKHZTTLY67JM3AVCNFSM6AAAAABGQO23AKVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDANZVGI4TMOJSGI . You are receiving this because you were mentioned.Message ID: @.***>