Add crop to landuse

[rwrx]

Add crop to landuse

[nvkelso]

I'm fine including this at zoom 15+, but not at earlier zooms as it'll prevent landuse merging.

https://wiki.openstreetmap.org/wiki/Key:crop

So in addition to adding it, you need to drop it at lower zooms.

And we should document the typical values (and even whitelist them).

[rwrx]

Ok, sure, but I am not sure how to add write it so it will be included only in zoom 15+. How can I do this?

[nvkelso]

I anticipate adding comments to this and other PRs on Wednesday.

[rwrx]

Ok, I understand. Thank you a lot for your patience with me :).

[nvkelso]

You can drop from low zooms by adding crop to the list of properties here:

• https://github.com/tilezen/vector-datasource/blob/master/queries.yaml#L895-L904

[nvkelso]

Whitelisting and remapping values:

Use this for example of whitelisting AND remapping values:

• https://github.com/tilezen/vector-datasource/blob/master/yaml/pois.yaml#L323-L380
• Follow that pattern declaring it up top in the "globals" section, and then include it in your new &crop_kind_detail definition.
• Do for the following (src) landuse kind only:
  • landuse=farmland

For crop values that have at least 200 instances (per TagInfo), and aren't funky:

value	count
rice	60 717
grape	51 229
grass	20 661
corn	16 743
cereal	12 177
tea	11 740
hop	11 034
sugarcane	10 735
market_gardening	7 476
field_cropland	6 769
wheat	6 460
soy	4 375
forage	3 931
barley	2 315
vegetable	2 266
rape	1 796
native_pasture	1 515
fast_growing_wood	1 228
coffee	1 089
lavender	730
beet	584
cotton	477
tobacco	299
sugar_beet	299
sunflower	268
hay	240
betel	192
rye	wiki
cassava	wiki
potato	wiki
asparagus	wiki
berries	wiki
cranberries	wiki
nut	wiki
flowers	wiki
protein	wiki

Ones that are funky and need to be remapped (using the WHEN syntax in the above example):

value	count	remap_to
cana-de-açúcar	5 923	sugarcane
vegetables	1 048	vegetable
cana-de-açucar	549	sugarcane
maize	147	corn
potatoes	wiki	potato

Some of the other ones are really other landuse types (eg orchard with tree tags), we'll ignore them completely and treat them as untrapped tagging errors.

[nvkelso]

Please see comments about adding crop as kind_detail on farmland, and remapping.

Also add test and documentation.

[nvkelso]

For test, use this one as a pattern:

vector-datasource/integration-test/1588-quay-wharf-other-sort-rank.py

Lines 63 to 86 in 5a64d33

	class MilitaryRangeTest(FixtureTest):
	def test_military_range(self):
	import dsl
	z, x, y = (16, 32645, 21849)
	self.generate_fixtures(
	# https://www.openstreetmap.org/way/28687274
	dsl.way(28687274, dsl.tile_box(z, x, y), {
	'description': u'Firing range',
	'landuse': u'military',
	'military': u'range',
	'name': u'No 4 Range',
	'source': u'openstreetmap.org',
	'source:name': u'os_street_view',
	}),
	)
	self.assert_has_feature(
	z, x, y, 'landuse', {
	'id': 28687274,
	'kind': 'range',
	})

Here's a legit rice paddy:

• https://www.openstreetmap.org/way/145660534

So it'd be more like:

# -*- encoding: utf-8 -*-
from . import FixtureTest

class FarmlandCropTest(FixtureTest):

    def test_crop(self):
        import dsl

        z, x, y = (16, 10631, 25149)

        self.generate_fixtures(
            # https://www.openstreetmap.org/way/145660534
            dsl.way(145660534, dsl.tile_box(z, x, y), {
                'landuse': u'farmland',
                'crop': u'rice',
                'source': u'openstreetmap.org',
            }),
        )

        self.assert_has_feature(
            z, x, y, 'landuse', {
                'id': 145660534,
                'kind': 'farmland',
                'crop': 'rice',
            })

    def test_crop_remap_sugarcane(self):
        import dsl

        z, x, y = (16, 10631, 25149)

        self.generate_fixtures(
            # https://www.openstreetmap.org/way/145660534
            dsl.way(145660534, dsl.tile_box(z, x, y), {
                'landuse': u'farmland',
                'crop': u'cana-de-açúcar',
                'source': u'openstreetmap.org',
            }),
        )

        self.assert_has_feature(
            z, x, y, 'landuse', {
                'id': 145660534,
                'kind': 'farmland',
                'crop': 'sugarcane',
            })

[rwrx]

Hopefully I have changed it as you requested.

[nvkelso]

The CI errors are related to:

File "/home/circleci/project/vectordatasource/meta/python.py", line 207, in ast_value
raise Exception("Don't understand AST value of %r" % val)
Exception: Don't understand AST value of u'cana-de-a\xe7ucar'

We likely need to escape that value.

[rwrx]

Hm actually, I am not sure how to do this in Python.

[rmarianski]

The rest of the string inputs are unicode right? Maybe this would work?

'cana-de-açúcar'.decode('utf-8')

[rwrx]

Hi @rmarianski thank you, I have changed it, but still the same output in the Circle.

[nvkelso]

Hmm, the escaping didn't work. @rmarianski @iandees any suggestions here?

There's a decent number of usages for those 2 forms so it'd be nice to include their remapping here.

[nvkelso]

Says flake8:

./integration-test/1588-quay-wharf-other-sort-rank.py:49:1: E302 expected 2 blank lines, found 1

[nvkelso]

Good news: tests run now!

Bad news:

1. There are unrelated OSM data changes causing some other existing tests to fail. That's beyond the scope of this PR but should be addressed before next release.
2. The new test was for crop instead of kind_detail, I've fixed that in fd9b39c.
3. Really the new crop tests should be in its own test file.

Overall, we're almost there. I'll let Circle run tests again to confirm the updated test works.

[rwrx]

Ok, I can create a new test file, but could I ask you for explanation of the rules for creating names for test files? As of now, I don't know how to name that file.

[nvkelso]

The new test passed! (We can ignore the unrelated test failures here)

To make a new file, just name it: 1926-landuse-crop-kind-detail.py in the same directory.

The general convention is to name it after the issue or PR number, and some descriptive text.

[rwrx]

@nvkelso Done. But tests are failing, I suspect that this is what you explained in previous comment:

"There are unrelated OSM data changes causing some other existing tests to fail. That's beyond the scope of this PR but should be addressed before next release."

[rwrx]

@nvkelso is there something I can fix here in order for this PR to be merged?

[nvkelso]

@rwrx Good to hear from you!

Most our changes in the v1.9 milestone have been focused on reducing tile size and extending support for disputed boundaries.

This PR would unfortunately increase tile size. I'd like to get it merged, but I think it needs to be paired with #2025, which would split all the "natural" landuse kinds over to it's own dedicate layer (which is also a breaking v2.0 change) so the size increases could be isolated and optional.

In the meantime, can you bring this branch up to date, please? That'll make it much easier to reason about – I can review w/r/t the low and mid zoom tile size reduction configs, and I can extend #2025 to also consider this PR.

[rwrx]

@nvkelso thank you a lot. I have updated this branch. Also I have questions about these PRs:
#1927
#1928
#1929
#1930
#1931
#1932

Thank you.

[nvkelso]

I left comments in the other PRs. Thanks for dusting them off! :)

[rwrx]

@nvkelso thank you. Should I close this PR in favor of #2025 when you will include changes from this PR into it?

[nvkelso]

No, please update this PR so it's mergeable after v1.9 is tagged and before v2.0 is tagged. Then #2025 will be limited in scope to a refactor instead of also bringing new features in.

[rwrx]

I have updated this PR. I hope that now it is ok.