-
Notifications
You must be signed in to change notification settings - Fork 27
Recreate iron mapping functionality #413
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
base: develop
Are you sure you want to change the base?
Conversation
Merge latest from develop into iron_mapping
Merge latest from develop into iron_mapping
Merge latest from develop into iron_mapping
…d supplemented in code comments
…nctionalized repeat code"
|
This is in draft status, still need to add a /docs page, add pytest, and double check changes necessary for geopandas and contextily install process (update main readme.md) Will up the description of the PR tomorrow |
…ommands for geopandas and contextily
…it call out of colorbar_limits
…nction to auto set the colorbar and colormap limits if None provided, passing plt.Normalize object to both the colormap and colorbar objects so both are normalized to the same range
…corrected environment.yaml and readme.md accordingly
|
Ok, I think this is ready for review. A couple of thoughts for those who review this:
SIDE NOTE: on the pyproject.toml, there are a few duplicates that could be cleaned up, ie: i see geopy in there atleast 3 times.
|
Co-authored-by: Jonathan Martin <94018654+jmartin4nrel@users.noreply.github.com>
Co-authored-by: Jonathan Martin <94018654+jmartin4nrel@users.noreply.github.com>
Co-authored-by: Jonathan Martin <94018654+jmartin4nrel@users.noreply.github.com>
We can add this detail to the issue once created, I'll do that this week. Given the discovery on these SSL errors in general, this is more so an issue at the OS / virtual environment level where not all libraries / packages use the same .pem / SSL certification file because they install their own (ie: requests, certifi, openssl, etc) and is not specific to the use of contextily & API calls to get tile basemaps (in this case from openstreetmap), I would vote the installation of pip-system-certs lives in the base dependencies in the pyproject.toml so these types of errors are auto caught / resolved for other packages that may have similar issues. |
@dakotaramos - I think that it'd be great to have tests for these individual functions if its not a big lift. These could live in |
elenya-grant
left a comment
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.
Looks good Dakota! I left a few notes in mapping.py but they're all just light suggestions/ideas and don't require any changes at this time. Thanks for all the great work on this! This is really great!
| colorbar_orientation (str, optional): A string used to set the orientation of the colorbar. | ||
| Defaults to 'horizontal'. | ||
| See matplotlib.pyplot.colorbar orientation parameter documentation for more info. | ||
| colorbar_limits (tuple | None, optional): A tuple used to manually set the lower and upper |
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.
I think in the future (not necessary for this PR) we could add a new variable like norm_type (right now it seems to default to a BoundaryNorm, which is likely the most common use-case) and then expand/update the colorbar_limits to handle different inputs for different norm types.
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.
I'd be curious to learn more about other norm types, to my knowledge the normalization that is happening is normalizing the colormap variable (LCOE, LCOI, etc) between [0.0,1.0] such that it can be mapped to the colormap color scheme. But definitely open to the idea if there are applicable use cases for it!
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.
Leaving as is for now and will keep this unresolved as reference for future PRs.
| Defaults to 0.8. | ||
| marker (str, optional): A string used to set the gpd.GeoDataFrame.plot() marker parameter. | ||
| Defaults to 's' == 'square'. | ||
| markersize (float, optional): A float used to set the gpd.GeoDataFrame.plot() markersize |
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.
Not-necessary for this PR but I've before made a function that takes a reference marker size and figure size to auto-size the marker size if the fig size changes. A marker size of 36 may look good on the default figure size but may appear way too small on a figure that's 2x the size. This kind of auto-calc could be useful to prevent having to fine-tune that input parameter but not necessary for this PR.
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.
Yea I'm all for more automation in this workflow for stuff like this, if you have already made some similar functionality feel free to add on in a future PR.
| The value represents a fraction of the height, or latitude range, the data points cover. | ||
| Example: if the data points span a distance of 1000 meters North-South, the basemap will | ||
| extend an additional (1000*0.05) = 50 meters down / south. | ||
| basemap_provider (xyzservices.TileProvider, optional): An xyzservices.TileProvider option |
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.
Is basemap_provider how we specify the underlying map or is that done with basemap_provider and another variable?
When I've made map-plots in the past, I normally just use Census geodata files to plot the outlines of states (and counties if wanted). In the future, I wonder if we could offer an option to use geodata files that is user-provided or pulled from open-source datasets like the census. This is just a thought - no changes necessary!
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.
basemap_provider is the contextily object that interfaces with the xyzservices.TileProvider API to pull in the underlying basemap .pngs to the extent and zoom / detail level needed based on the extent (lat/long range) of the data. It is the main variable for specifying but there are some things under the hood that contextily does to interface with the geopandas plot to add the basemap.
I do see value in having the ability to use state outlines (ie: census TIGER state.zip files).
Can you share an existing workflow / code that you have that plots points on a state outline map, for my own reference? I like the idea of having that as an option instead of contextily (especially with all the SSL errors around that), but I would have to play around with the workflow a bit to make sure it works smoothly. Right now contextily auto detects the current gdf / plot and adds to that, with the state outline maps that may need to be added as the basemap at the beginning? TBD.
| | tuple[gpd.GeoDataFrame, ...] | ||
| | None = None, | ||
| show_plot: bool = False, | ||
| save_plot_fpath: Path | str | None = None, |
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.
I'm not sure whether this should go in the configuration class or as an input to the function, but its sometimes nice to be able to specify the dpi (I think the default is 100 or 200) if saving a figure:
if Path(save_plot_fpath).suffix != ".pdf":
fig.savefig(save_plot_fpath,bbox_inches="tight",dpi=save_plot_dpi)
else:
fig.savefig(save_plot_fpath,bbox_inches="tight")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.
Yea it would be good to have atleast the format and dpi arguments.
Might be worth making a savefig_preference object in the config class for all the keyword args https://matplotlib.org/stable/api/_as_gen/matplotlib.pyplot.savefig.html
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.
Feel free to copy/take inspiration from some of the work I've done in the OpenOA plotting module that falls into the same category, just don't repeat my unfixed mutable defaults that I've had you fix in this PR. The major difference from what you've proposed is that if a user provides a bad argument to matplotlib, it's on them, not us to identify.
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.
I think for now I'm simply going to add a save_plot_dpi argument to the functions, but would like to revisit this in future PRs. It almost sounds like this could be part of Elenya's previously suggested PlotOptionsBaseConfig or even its own SavefigOptionsBaseConfig class
I would recommend against adding this library to the H2Integrate dependency stack because it's not an H2I problem, but a corporate security layer problem. Some other institutional users may encounter these issues, but much of our errors stem from the fact that we're operating on a VPN with certifications that don't always play nicely. |
I believe all of Rob's comments have been addressed, dismissing this review to allow for merging
Copy that, I'll be sure to include this sentiment in the issue that is created along with all of the solutions we've found for the different operating systems |
@elenya-grant added in some tests for the helper functions, if you can review and let me know if there is anything else you would add, thanks! |
Recreating iron mapping functionality with geopandas, matplotlib, and contextily
Recreating the iron mapping functionality from previous usa_iron repo. Leverages Geopandas, Matplotlib, and Contextily to plot geospatial point heat maps and simple straight line shipping / transport routes with multiple layers and open source basemaps. Core functionality added to
/h2integrate/postprocess/mapping.py, open to changing file and function names.GeospatialMapConfigconfig class to handle validation and default values for all parameters needed to format the plots inplot_geospatial_point_heat_map()andplot_straight_line_shipping_routes()plot_geospatial_point_heat_map()function to create or add layers to a geospatial point heat map. Functionally this plots a point heat map across a range of sites (lat,long) with the color map indicates the value of a metric of interest (ie: AEP, LCOE, LCOI, etc.) with an openstreetmap (or other) basemap.plot_straight_line_shipping_routes()function to plot straight line shipping routes. Can either be used to create or add layers to an existing map. NOTE: this was developed for proof of concept work for the ITO Iron electrowinning project, this is a very simplified method for plotting straight line routes, I anticipate this will be revamped or replaced entirely with more complex transport / shipping is developed or integrated into H2I in the future.calculate_geodataframe_total_bounds()helper function to determine the total bounds / extent (range of lat/long) the data covers in a single or set of Geopandas GeoDataFrames.auto_detect_lat_long_columns()helper function to automatically detect which columns in a Pandas DataFrame represent the latitude and longitude of a given site. Used if the user does not provide the column names as arguments inplot_geospatial_point_heat_map()andplot_straight_line_shipping_routes()validate_gdfs_are_same_crs()helper function to validate if 2+ geodataframes are using the same coordinate reference system (CRS)auto_colorbar_limits()helper function to automatically set visually please / even limits for the colorbar legends, this is also used to set the limits for normalizing the colormap and colorbar. Users can also specify the limits manually in the map_preferences dictionary (colorbar_limits key)/examples/26_iron_map/contains standard config files with additional csvs (with some dummy data for ore and shipping costs) used inrun_iron.py. Lines 15 onward show an example flow of how this would be used to add 3 heatmap layers (LCOI, LCO iron ore, waterway shipping costs in $/tonne-X) and 3 straight line shipping route layers and saves the plot in in the same location as the cases.sql the design of experiment H2I runs produces/examples/26_iron_map/ex_26_out/docs/user_guide/plotting_geospatial_data_with_geopandas.mdsimple workflow of how to use this code and an example figure outputType of Contribution
General PR Checklist
CHANGELOG.mdhas been updated to describe the changes made in this PRdocs/files are up-to-date, or added when necessaryNew Technology Checklist
test_all_examples.pyexamples/README.mddocs/technology_models/docs/technology_models/technology_overview.mdsupported_models.pycreate_financial_modelinh2integrate_model.pydocs/developer_guide/coding_guidelines.mdRelated issues
#300
Impacted areas of the software
path/to/file.extensionmethod1: What and why something was changed in one sentence or less.Additional supporting information
Test results, if applicable