River bug

[barneydobson]

Description

Crashes when no rivers inside the bbox. Though this is fine now that we have option to use lowest elevation point.

Now fixed and tested

[cheginit]

I recommend adding retain_all=True for getting rivers, since by default it will only keep the largest connected network, which is fine for streets but not necessarily for other network types.

[barneydobson]

huh, didn't realise that - thanks!

[cheginit]

When using pytest, exceptions can be checked like so:

def test_no_river():
    bbox = (0.0402, 51.55759, 0.09825591114207548, 51.6205)
    with pytest.raises(ValueError, match="Found no graph nodes"):
        _ = ox.graph_from_bbox(bbox)
    G = downloaders.download_river(bbox)
    assert G.size() == 0

[barneydobson]

Yep - though that will actually run the download which the testing module aims to avoid - I will add this test with @pytest.mark.downloads - these are separate tests to test the actual downloads.

[barneydobson]

Hmm I'm not sure I fully understand actually - this test isn't checking an exception. The graph_from_bbox will raise an exception inside download_river, causing it to return an empty graph - which is what the test is checking.

[cheginit]

My understanding of the test was that you want to first check if the ox.graph_from_bbox indeed raises ValueError and then run the same with downloaders.download_river and check if it handles the exception correctly and returns an empty graph.

[cheginit]

Regarding getting data with osmnx, I recommend enabling its caching. You can set it with osmnx.settings.use_cache = True

[barneydobson]

now in issue #183

[dalonsoa]

Looks good! Just agreed with @cheginit comment on using pytest features to test exceptions.