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

Halfords #2144

Merged
merged 4 commits into from
Oct 25, 2018
Merged

Halfords #2144

merged 4 commits into from
Oct 25, 2018

Conversation

eigenbrot
Copy link
Collaborator

Two things done here, one easy and one not so cut-and-dry:

  1. Halfords Autocentre is clearly a related but separate brand from Halfords. They have individual wikipedia and wikidata entries. To minimize confusion I enforced the canonical car_repair|Halfords Autocentre over car_repair|Halfords, which had slightly more matches.

  2. The brand Halfords is a little trickier. According to the wikidata this is a car_parts store. But from wikipedia and their website it is clear that they also sell bicycles and camping gear. Furthermore, there are more matches for bicycle|Halfords than car_parts|Halfords. In the current PR I've made car_parts the canonical category and have it match to bicycle, but clearly more OSM users view this as a bicycle shop so I'm not sure this is correct. Halfords' own website title bar even lists "Bikes, Cycling" before car parts.

I also had the thought of adding a match for shop/outdoor|Halfords as a preliminary strike on someone using that tag after they buy a tent or whatever. This is currently not in the PR because I'm worried it might be too cute or convoluted. Opinions welcome!

car_repair|Halfords had (slightly) more matches, but "Halfords" is an affiliated
store that does NOT offer car repair, so I fell back on the offical name of this
brand, "Halfords Autocentre".
This is a related, but separate, brand from Halfords Autocentre.

Not sure that choice of car_parts over bicycle was correct; this store seems to
sell everything!
@eigenbrot eigenbrot added the question Not Actionable - just a question about something label Oct 24, 2018
@matkoniecz
Copy link
Contributor

matkoniecz commented Oct 24, 2018

There is (frequently proposed, rarely used ) sells:* tag scheme. In taginfo I also found service:bicycle:retail (5k uses) and service:bicycle:sales.

Maybe add service:bicycle:retail=yes, sells:bicycles, sells:car_parts tags to the preset?

@eigenbrot
Copy link
Collaborator Author

@matkoniecz, that sounds like it might be a good solution. I'm afraid I'm not familiar with any of those tags you referenced, though, and could not find any examples in canonical.json. Are they just added to the tags section for a particular brand?

@matkoniecz
Copy link
Contributor

Are they just added to the tags section for a particular brand?

Yes - tags may contain any tags that will be applied -

"cuisine": "burger",

I'm afraid I'm not familiar with any of those tags you referenced

https://wiki.openstreetmap.org/wiki/Key:service:bicycle:retail

sells tag family is described https://wiki.openstreetmap.org/wiki/Proposed_features/Sells and has very limited use, but in this case I see no better way of tagging that

@matkoniecz
Copy link
Contributor

BTW, in tricky cases like this one I would also consult local community where this shop appears frequently. Maybe they will have some good ideas/preferences.

@eigenbrot
Copy link
Collaborator Author

Thanks for the links, @matkoniecz! That first one has an example that feels eerily relevant :).

I tried adding sells: bicycles (pluralization matters?), sells:car_parts (probably redundant), and sells:outdoors (maybe should be outdoors:camping?), but npm run build seems to only allow a single sells tag. Is this expected?

@eigenbrot
Copy link
Collaborator Author

Oh wait, perhaps the solution is to put them all into a list:

sells: ["bicycle", "car_parts", "outdoors"]

Is that right? It at least gets left alone by npm run build. Thanks for dealing with my n00bish questions!

@bhousel
Copy link
Member

bhousel commented Oct 25, 2018

Is that right? It at least gets left alone by npm run build. Thanks for dealing with my n00bish questions!

Nah, I'd skip the sells tags. These aren't very well established in OpenStreetMap yet.

@eigenbrot
Copy link
Collaborator Author

OK, I removed the sells tag and pushed a commit that just adds "service:bicycle:retail": true. Is that good enough?

Also, related to @matkoniecz comment about local community, I did a bunch of Googling and Halfords is often referred to as a big box store that sells car and bike stuff. That said, all of their official pages (twitter, facebook, etc.) list the car parts first.

@matkoniecz
Copy link
Contributor

matkoniecz commented Oct 25, 2018

"service:bicycle:retail": true,

I think it should be

"service:bicycle:retail": "yes",

true may end getting converted to "yes" string but I would not rely on that.

@matkoniecz
Copy link
Contributor

matkoniecz commented Oct 25, 2018

I tried adding sells: bicycles (pluralization matters?), sells:car_parts (probably redundant), and sells:outdoors (maybe should be outdoors:camping?), but npm run build seems to only allow a single sells tag. Is this expected?

sells idea was nixed, but it would be

"sells:car_parts": "yes",
"sells:bicycles": "yes",
"sells:whatever_else": "yes",
"sells:yet_another_key": "yes",

in tags array.

@eigenbrot
Copy link
Collaborator Author

I think it should be

"service:bicycle:retail": "yes",

true may end getting converted to "yes" string but I would not rely on that.

Is this true?(hehe) I can find no instances of "key": "yes" in canonical.json but many cases of "key": true (e.g., "nocount": true). I have no experience with json so I'm very willing to be proven incorrect!

@matkoniecz
Copy link
Contributor

matkoniecz commented Oct 25, 2018

Is this true?

Yes :)

These are about a bit different things. "nocount": true is internal data (instruction to ignore one of checks during validation), "service:bicycle:retail": "yes", is definition of tag that will be added to OSM database.

I can find no instances of "key": "yes" in canonical.json

Probably it is the first boolean OSM tag added to the brand dataset. Note that most entries have just name, brand, brand:wikipedia and brand:wikidata in tags array. Additional OSM tags are quite rare.

@eigenbrot
Copy link
Collaborator Author

OK, @matkoniecz thanks for the clarification! I've made that change in the most recent commit.

Phew, I think this might be ready for a merge!

@matkoniecz
Copy link
Contributor

One more thought: it may look confusing for user as "bicycles are also sold here" will not be well visible for example in iD as user selects object. But I am not sure how it may be fixed, keep both shop=bicycle sells:car_parts=yes and shop=car_parts service:bicycle:retail=yes?

@matkoniecz matkoniecz merged commit 076f39d into osmlab:master Oct 25, 2018
@matkoniecz
Copy link
Contributor

@eigenbrot Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
question Not Actionable - just a question about something
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants