-
-
Notifications
You must be signed in to change notification settings - Fork 363
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
Docs clean up #800
Docs clean up #800
Conversation
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, thanks!
on: | ||
tags: true | ||
all_branches: true | ||
|
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.
It looks like there is no longer an obvious way to build the docs locally? I do want them to be built by CI in the typical case, but the point of using dodo.py was to make it simple to run the same command that CI runs, so that both local and CI builds are based on the same spec.
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 the problem is that with dodo the output is all dumped out at the end, so travis can time out and it is really hard to tell what is going on.
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.
Ugh.
Ok so there are the image bounds errors and then there is the fact that geography takes more than 10 min to run. I am finding this locally as well. Is there any way it could be meaningfully split up or the data size reduced? @brendancol @jbednar |
I'm not sure what image bounds errors you're referring to, but the images could definitely be reduced in size, which should speed things up proportionally. Maybe reduce by 30% in width, which should cut the time in half? |
I should have separated out those comments. There are two separate thing going on. The image bounds errors I'm getting look like: ~/miniconda/envs/3.6/lib/python3.6/site-packages/holoviews/element/raster.py in __init__(self, data, kdims, vdims, bounds, extents, xdensity, ydensity, rtol, **params)
323 'density.')
324 SheetCoordinateSystem.__init__(self, bounds, xdensity, ydensity)
--> 325 self._validate(data_bounds, supplied_bounds)
326
327
~/miniconda/envs/3.6/lib/python3.6/site-packages/holoviews/element/raster.py in _validate(self, data_bounds, supplied_bounds)
391 not_close = True
392 if not_close:
--> 393 raise ValueError('Supplied Image bounds do not match the coordinates defined '
394 'in the data. Bounds only have to be declared if no coordinates '
395 'are supplied, otherwise they must match the data. To change '
ValueError: Supplied Image bounds do not match the coordinates defined in the data. Bounds only have to be declared if no coordinates are supplied, otherwise they must match the data. To change the displayed extents set the range on the x- and y-dimensions. See https://travis-ci.org/pyviz/datashader/jobs/593702566#L1830-L2113 for full output |
Right, I'm only answering the "10 min" issue, not the image bounds issue, which I don't know anything about. To speed it up, you should be able to reduce the image size, because many of the calculations (e.g. distance) are done once per pixel. |
holoviz/holoviews#4035 should fix the image bounds error. |
Co-Authored-By: James A. Bednar <jbednar@users.noreply.github.com>
I tried trimming down the size of the image for the geography notebook, but the viewshed part still takes a really long time (minutes). Could something have changed to make this very very slow? |
Scratch that. I hadn't realized terrain gets set several times over the course of the notebook. |
0cd5583
to
d903d0a
Compare
Sorry for the pings Brendan. |
I think this can be merged. https://pyviz-dev.github.io/datashader/user_guide/Trimesh.html is still broken until the next holoviews tag, but that is minor. |
This PR:
It does not: