Allow custom geojson files in create_map

[bowguy]

Add two additional parameters in create_map to allow custom GeoJSON files. Existing examples and usage unchanged.

[aerispaha]

@bowguy thanks for this contribution! Overall, I'm in favor of making this kind of change to the create_map function, as I can see how this would be useful.

That said, there are several things about this function that I think could be improved, unrelated to your contribution. You're following the patterns in the function, which is great. But the patterns I wrote many moons ago aren't so great. These are some of the things that give me pause:

1. There is no type hinting, which makes it tricky for users to know what to types to pass into this function (this is an issue throughout most of swmmio, to be address under add type hints on all the public functions/methods #240)
2. The arguments should follow snake case per PEP8, but only one of them does (the one that isn't even used in the function)
3. The arguments should be more consistent with the draw_model function, at least where there is overlap. (e.g. there is a filename argument in one and a file_path argument in the other) 🫠

So, before we make this change, I'd prefer to clean up some of these things. I propose that we do the following:

1. sync your branch with the latest in master (this will include a more complete docstring for this function)
2. expand the docstring to include these new optional arguments
3. add type hints
4. refactor the function to use the following arguments and types:
   • model : swmmio.Model, optional
   • nodes : geopandas.GeoDataFrame or GeoJSON string, optional
   • links : geopandas.GeoDataFrame or GeoJSON string, optional
   • file_path : string or Path, optional
   • basemap : string or Path, optional

What do you think? I'm open to suggestions, I just want to make sure we improving the user experience while we add new features.

[bowguy]

I appreciate all your comments on this. Some of my thoughts

• Something needs to be tweaked. Right now create_map generates model results for the links but not the nodes. It only generates some configuration info of the nodes. See https://github.com/orgs/pyswmm/discussions/225
• I think we need to address nodes and storage nodes separately. They are handled differently in the results. For me, I have large regional detention ponds that are the driving design points.
• I like the idea of passing a geojson file or a dataframe. Should be easy to test for the type sent.

Maybe I should cancel my pull request and tackle things one step at a time. Obviously, the documentation will need to be fleshed out. Are you thinking about replacing create_map with a new function?

[aerispaha]

Right now create_map generates model results for the links but not the nodes

Wow I didn't realize that (or forgot).

Overall I agree with you that something needs to be tweaked, and I think it makes sense to address the nodes and storage nodes issues separately.

I suggest we keep this PR and focus it on

1. adding type hints
2. completing the docstring to include the new optional arguments (whose type can be a geojson file or a dataframe, if you're up for developing that logic)
3. refactoring the function arguments as I described above.

Then separately we can work on the nodes and storages issues you've identified. Is that okay with you?

Are you thinking about replacing create_map with a new function?

No, I don't think so. But I am interested in making this functionality available as a method on the core Model object. But that's not a critical issue IMO - something that we can consider separately.

[bowguy]

Closed this due to the confusion. New pull request submitted.