-
Notifications
You must be signed in to change notification settings - Fork 24
Fixed dh issue by modifying get_bbox() #175
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
Conversation
Codecov Report
@@ Coverage Diff @@
## v0.6.0-rc #175 +/- ##
============================================
Coverage ? 54.96%
============================================
Files ? 20
Lines ? 3495
Branches ? 552
============================================
Hits ? 1921
Misses ? 1453
Partials ? 121 Continue to review full report at Codecov.
|
csep/core/forecasts.py
Outdated
@@ -437,7 +437,7 @@ def plot(self, ax=None, show=False, log=True, extent=None, set_global=False, plo | |||
axes: matplotlib.Axes.axes | |||
""" | |||
# no mutable function arguments | |||
dh = round(self.region.dh, 5) | |||
# dh = round(self.region.dh, 5) |
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.
can you remove these comments?
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 removing now.
csep/utils/plots.py
Outdated
@@ -862,7 +862,7 @@ def plot_spatial_dataset(gridded, region, ax=None, show=False, extent=None, set_ | |||
# Get spatial information for plotting | |||
bbox = region.get_bbox() | |||
if extent is None and not set_global: | |||
extent = [bbox[0], bbox[1], bbox[2] + region.dh, bbox[3] + region.dh] | |||
extent = [bbox[0], bbox[1], bbox[2], bbox[3]] # extent = [bbox[0], bbox[1], bbox[2] + region.dh, bbox[3] + region.dh] |
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.
unnecessary comment here as well
csep/utils/plots.py
Outdated
# lons, lats = numpy.meshgrid(numpy.append(region.xs, region.xs[-1] + region.dh), | ||
# numpy.append(region.ys, region.ys[-1] + region.dh)) |
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.
here as well
@khawajasim looks good! i made a couple comments about some comments. other than that it looks good to go! this pr fixes #166 |
Thanks for the merge. When is it going into the master branch now? I would like to add a few more functions in the coming days, mainly related to forecast mapping from one grid to another. Although, those functions will not be part of the QuadtreeGrid2D class. But generally part of region.m file. I want to see this through first and then add further. Cheers, |
* Quadtree region class - QuadtreeGrid2D (#115) * Added function to read QuadtreeGrid forecast * added spatial_count and spatio_mag_count functions * poission_evaluations using from QuadtreeGrid2D * unit tests for quadtree-grid * California quadtreegrid at L=12 added * california quadtreegrid loading function * setup.py modified-added mercantile * forecast.plot() for Single-res Quadtree * clean up unnecessary comments from code * Update python-app.yml (#170) * added documentation for roc and i1 score (#169) * Fixed dh issue by modifying get_bbox() (#175) Co-authored-by: khawajasim <asimkhawaja786@gmail.com> Co-authored-by: William Savran <wsavran@usc.edu>
Fixes issue #166