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

new quest: adding religion to place_of_worship, fixes #777 #802

Merged
merged 9 commits into from Feb 4, 2018

Conversation

matkoniecz
Copy link
Member

@matkoniecz matkoniecz commented Jan 26, 2018

this quest has high priority as it is both relatively rare and used by major data consumers in a prominent way

For now statistics about religions are from OSM itself. I did it for following reasons

  • this way countries without real OSM avrtivity are skipped
  • it automatically handles cases of religions maintaining more/less place of worship than indicated by its religion share
  • it reflects interest of OSM editors in region
  • note that as result of factors above it may significantly differ from official religion distribution
  • data should be easy to update by rerunning the script
  • I was curious how it may be done and making statistics from OSM data was more interesting than manually collecting data or parsing official data

Open issues

  • I think that it would be a good idea to extract code shared by AddSportForm and formm for this quest. AbstractAddAttributeForm? (either as separate PR or by adding commit to this one)
  • I wonder how script used to generate this data should be shared, I guess that link to file in a separate repo linked fom popularReligion.yml header would be the best idea (requires updating this PR to include link)
  • AFAIK it is the first attempt to directly transform OSM data into country data files - hopefully it is OK
    • if for some reason data for all countries is needed, even ones without OSM editors I may try parsing some statistics like one from add missing religion tag to amenity=place_of_worship #777 (comment)
    • using official statistics would also be necessary if order on list is supposed to follow official statistics, rather how many amenity=place_of_worship of given religion are mapped in a given country

@matkoniecz matkoniecz force-pushed the religion branch 3 times, most recently from 774ab67 to 651a232 Compare January 27, 2018 00:09
@matkoniecz matkoniecz changed the title new quest: adding religion to place_of_worship new quest: adding religion to place_of_worship, fixes #777 Jan 27, 2018
Copy link
Member

@westnordost westnordost left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The research for popular religions is not that good. I might look into that tomorrow myself.

return "nodes, ways, relations with amenity=place_of_worship and" +
" !religion and" +
" name and" +
" (access !~ private|no)"; // exclude ones without access to general public
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You'd still be able to see it from the outside.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I thought about edge cases like PoW in a military base or a prison. Are you sure that it should be dropped?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

dropped

public class AddReligionToPlaceOfWorshipForm extends ImageListQuestAnswerFragment
{
private static final int
MAX_DISPLAYED_ITEMS = 24, INITIALLY_DISPLAYED_ITEMS = 8;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this is unnecessary. There are less than 24 items and getMaxNumberOfInitiallyShownItems could just return 8.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, it should be dropped in case of extracting only createItems rather than making an abstract class/

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done

IL: [jewish, christian, muslim]
IN: [hindu, christian, muslim, buddhist, sikh, jain]
IQ: [muslim]
IR: [muslim]
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You know where Bahai Faith is from?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

According to Wikipedia

It initially grew in Iran (Persia) and other parts of the Middle East region, where it has faced ongoing persecution since its inception.[2] Currently it has between 5 and 7 million adherents, known as Bahá'ís, spread out into most of the world's countries and territories

Given persecution in their home area, spread across world and low total numbers it was not surprising that they are not appearing on list of any country (though it may be next weakness of OSM data).

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

https://en.wikipedia.org/wiki/Iran#Religion

according to statistics center of Iran, Bahais constitute only about 0.37% of Iran

last data mentioned in https://en.wikipedia.org/wiki/Bah%C3%A1%27%C3%AD_Faith_in_Iran is from 1920

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The first sentence of this article says "The Bahá'í Faith in Iran is the country's second-largest religion after Islam"

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

somehow I missed it, I added now bahai in Iran

@westnordost
Copy link
Member

I think that it would be a good idea to extract code shared by AddSportForm and formm for this quest. AbstractAddAttributeForm? (either as separate PR or by adding commit to this one)

I'd say that this sort method could be put into some static class as a static method.

@westnordost
Copy link
Member

For now statistics about religions are from OSM itself.

I'd rather use official religion statistics. OSM is very incomplete here.

@matkoniecz
Copy link
Member Author

I'd say that this sort method could be put into some static class as a static method.

Done.

@matkoniecz
Copy link
Member Author

Now only issue of religion data should remain...

OSM is very incomplete here

I agree, but it seems to me that OSM is incomplete in places that have no OSM editors including StreetComplete users.

The research for popular religions is not that good. I might look into that tomorrow myself.

Is data from http://data.un.org/Data.aspx?q=population+religion&d=POP&f=tableCode%3a28%3bsexCode%3a0&c=2,3,6,8,10,12,14,15,16&s=_countryEnglishNameOrderBy:asc,refYear:desc,areaCode:asc&v=1 significantly better?

@westnordost
Copy link
Member

Whoa, in what time zone do you live, if you are up at half past one on the one hand but also up at 5, 6,7 and 9 in the morning?

@westnordost
Copy link
Member

Is data from http://data.un.org/Data.aspx?q=population+religion&d=POP&f=tableCode%3a28%3bsexCode%3a0&c=2,3,6,8,10,12,14,15,16&s=_countryEnglishNameOrderBy:asc,refYear:desc,areaCode:asc&v=1 significantly better?

I don't know, it is a little too detailed and thus maybe hard and a lot of work to go through there. After all, the information we gather should just be used to change the order a bit. I'll search through wikipedia a bit now.

import de.westnordost.streetcomplete.view.Item;

public class PriorityList {
public static List<Item> BuildList(List<Item> allItems, List<String> popularNames){
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

According to usual Java naming conventions, functions always start with lowercase letters.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ops, fixed.

@matkoniecz
Copy link
Member Author

Whoa, in what time zone do you live

Unfortunately my timezone is not interesting (Poland) and what worse today there was a very interesting and loud noise outside on early morning.

@westnordost
Copy link
Member

westnordost commented Jan 27, 2018

hey, I am back. Spent the last hours to do some extensive research on wikipedia, mostly 💦

Shinto is Japanese folk religion. But another, more important folk religion was left out, Chinese folk religion (aka Chinese traditional religion, Han folk religion, Chinese popular religion, Sheinism), because I guess there are little sources on that.

Also, I have to say my first impression on the popularReligions.yml was wrong, it really for the most part coincides with my research.

@matkoniecz
Copy link
Member Author

So it is ready for merging (I know that it may wait for end of the feature freeze) or is there something to do?

BTW, I would recommend adding chinese_folk value to https://wiki.openstreetmap.org/wiki/Key:religion as currently it is neither listed there nor used even once in the OSM data (I am not planning to add this as I have no idea is it a good idea or not).

@westnordost
Copy link
Member

So it is ready for merging (I know that it may wait for end of the feature freeze) or is there something to do?

Well you forgot to export the metadata. You should test if the religion quest displays and works correctly though. I.e. also in a country where there is a different order than the usual one.

@matkoniecz
Copy link
Member Author

You should test if the religion quest displays and works correctly though.

I did it, including making some edits.

also in a country where there is a different order than the usual one

Unfortunately I failed to to this one. Would it make sense to include in CONTRIBUTING.md a checklist what should be done before making a PR?

@matkoniecz
Copy link
Member Author

I plan on retesting it in countries with nonstandard religion order (though this time it will be harder to make test edits - is there a better solution than making edit and reverting it with "sorry, test edit"?), but for now my SC install is locked by #812 (comment)

@westnordost
Copy link
Member

You can turn off auto upload and/or remove authorization so that the app cannot upload changes, then undo.

@matkoniecz
Copy link
Member Author

Or disable internet connection and uninstall app before enabling it (my method so far, undo one seems to be far better - thanks, I though that with undo edit + undo edit would still be send).

Anyway, is there some way to examine what edit would be made? I want to catch bugs noticeable only on examining edit made by app - 926a30f (adding building tag always collided with present building=yes, was fixed by rather modifying it)

In past I also had bug that resulted in incorrect changeset description, I am unsure how to catch this without making a real edit.

@westnordost
Copy link
Member

So tell me when this is tested and can be merged

matkoniecz and others added 9 commits February 3, 2018 08:05
this quest has high priority as it is both relatively rare and used by major data consumers in a prominent way
…lk, caodai, bahai, jews and sikh around the world (wikipedia)

- add Chinese folk religion
- sort standard order by worldwide distribution *minus* those already sorted in popularReligions.yml
- show only 4 items by default
- remove Confucianism: indistinguishable from Chinese folk religion / not really a religion
@matkoniecz
Copy link
Member Author

It is rebased and tested and I consider it ready for merge.

@westnordost westnordost merged commit e08e5b3 into streetcomplete:master Feb 4, 2018
@westnordost
Copy link
Member

merged!

@matkoniecz matkoniecz deleted the religion branch February 4, 2018 11:17
@matkoniecz
Copy link
Member Author

Thanks for review and merging!

@matkoniecz
Copy link
Member Author

Though, I would still recommend adding chinese_folk value to https://wiki.openstreetmap.org/wiki/Key:religion as currently it is neither listed there nor used even once in the OSM data (I am not planning to add this as I have no idea is it a good idea or not).

@westnordost
Copy link
Member

Ok

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants