Skip to content
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/viewshed testing #807

Merged
merged 7 commits into from Oct 15, 2019

Conversation

@brendancol
Copy link
Collaborator

commented Oct 9, 2019

  • small fixes to viewshed numba dtypes
  • adds better tests for viewshed accuracy
@brendancol brendancol self-assigned this Oct 9, 2019
@jbednar
jbednar approved these changes Oct 9, 2019
datashader/spatial/viewshed.py Outdated Show resolved Hide resolved
@brendancol

This comment has been minimized.

Copy link
Collaborator Author

commented Oct 10, 2019

@jbednar This is currently not passing the nbsmoke appveyor tests, but there were no changes to notebooks. It looks like appveyor is missing a holoviews dependency. is this a known issue?

@jbednar

This comment has been minimized.

Copy link
Member

commented Oct 10, 2019

datashader/spatial/viewshed.py Outdated Show resolved Hide resolved
@jsignell

This comment has been minimized.

Copy link
Member

commented Oct 10, 2019

This is currently not passing the nbsmoke appveyor tests, but there were no changes to notebooks. It looks like appveyor is missing a holoviews dependency. is this a known issue?

This is super strange. The deps are the same and don't include holoviews, but on travis holoviews gets pulled in and on appveyor holoviews doesn't. The only difference is the channel. I guess we can try getting rid of conda-forge on appveyor and see what happens.

@brendancol

This comment has been minimized.

Copy link
Collaborator Author

commented Oct 13, 2019

This is currently not passing the nbsmoke appveyor tests, but there were no changes to notebooks. It looks like appveyor is missing a holoviews dependency. is this a known issue?

This is super strange. The deps are the same and don't include holoviews, but on travis holoviews gets pulled in and on appveyor holoviews doesn't. The only difference is the channel. I guess we can try getting rid of conda-forge on appveyor and see what happens.

@jsignell cool cool. thanks for checking this out. I'll see if

This is currently not passing the nbsmoke appveyor tests, but there were no changes to notebooks. It looks like appveyor is missing a holoviews dependency. is this a known issue?

This is super strange. The deps are the same and don't include holoviews, but on travis holoviews gets pulled in and on appveyor holoviews doesn't. The only difference is the channel. I guess we can try getting rid of conda-forge on appveyor and see what happens.

Cool I will try this and revert if it doesnt work

@jsignell

This comment has been minimized.

Copy link
Member

commented Oct 14, 2019

Seems like the python 2.7 failures are legit.

UnsupportedParforsError: Failed in nopython mode pipeline (step: nopython mode backend)
The 'parallel' target is not currently supported on Windows operating systems when using Python 2.7, or on 32 bit hardware.
@jsignell

This comment has been minimized.

Copy link
Member

commented Oct 14, 2019

Probably not caused by this PR, but legit.

@jbednar

This comment has been minimized.

Copy link
Member

commented Oct 15, 2019

I'll go ahead and merge this even though I think we do need py27 tests, but apparently they shouldn't be on Appveyor anyway, because of limitations in Numba's parfors support on Windows. Meanwhile, there do appear to be appveyor issues unrelated to this PR, but they can be addressed in a separate PR.

@jbednar jbednar merged commit 3d0dcb1 into pyviz:master Oct 15, 2019
1 of 2 checks passed
1 of 2 checks passed
continuous-integration/appveyor/pr AppVeyor build failed
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
brendancol added a commit that referenced this pull request Oct 15, 2019
* fixes to numba types
* made tests more robust by adding viewshed correctness checks
* changed min gradient to -inf
* refactor PI variable into import alias
* removed python 2.7 appveyor testing
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants
You can’t perform that action at this time.