-
-
Notifications
You must be signed in to change notification settings - Fork 367
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
Brendancol/geo transfer functions #661
Brendancol/geo transfer functions #661
Conversation
…smoke test notebooks
On travis and appveyor, after installing datashader's dependencies using conda, a develop/editable install of datashader is done using pip ( I had this problem myself on another project, and fixed it temporarily by running |
"clearly an installed package with broken metadata shouldn't break pip like this" -- agreed! @brendancol, please do add that pin to see if it fixes the build or at least lets you go on to the next issue. ETA: Actually, I've pushed that fix in another PR, so if you rebase your PR on master, the pip problem should go away... |
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've added Algorithm References
section to each of the geo methods
|
||
if not agg.attrs.get('res'): | ||
#TODO: maybe monkey-patch a "res" attribute valueing unity is reasonable | ||
raise ValueError('input xarray must have numeric `res` attr.') |
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.
Shouldn't the docstring explain that res is required, and what it should be?
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've added the following note to the docstring:
Notes:
------
Input aggregate must contain a `res` (numeric) attribute which specifies the
x/y cellsize for use in calculating slope. It is assumed that cells are square
and have the same resolution in x/y dimensions.
Algorithm References:
- http://desktop.arcgis.com/en/arcmap/10.3/tools/spatial-analyst-toolbox/how-slope-works.htm
- Burrough, P. A., and McDonell, R. A., 1998. Principles of Geographical Information Systems (Oxford University Press, New York), pp 406
datashader/geo.py
Outdated
|
||
def mean(agg, passes=1, excludes=[np.nan]): | ||
""" | ||
Returns Mean filtered array using a 3x3 window sum number of times |
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.
"Returns mean filtered array using a 3x3 window applied in multiple passes"?
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.
Yes...revised...
"## Bump\n", | ||
"Bump mapping is a cartographic technique often used to create the appearance of trees or other land features.\n", | ||
"\n", | ||
"The `datashader.bump` will produce a bump aggregate that can then used to add detail to the terrain. In this case, I will pretend the bumps are trees and shade them with green." |
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 then be used
@jbednar I think I've hit all your review notes. CI is also now passing. |
Thanks! Now on to the tile generation! |
No description provided.