Skip to content

Omit religion=none from suggested options of religion field for places of worship#1940

Merged
tyrasd merged 5 commits intoopenstreetmap:mainfrom
ak8abhinay:fix-religion-values
Apr 7, 2026
Merged

Omit religion=none from suggested options of religion field for places of worship#1940
tyrasd merged 5 commits intoopenstreetmap:mainfrom
ak8abhinay:fix-religion-values

Conversation

@ak8abhinay
Copy link
Copy Markdown
Contributor

@ak8abhinay ak8abhinay commented Feb 12, 2026

This update follows the suggestion to stop promoting religion=none as a suggested value in the religion field.

Problem: Offering 'Non-religious' as a clickable option for 'Religious Area' or 'Place of Worship' presets created a logical contradiction.

Fix: By removing none from the options list in religion.json, the editor no longer 'offers' this value as a button.

Existing data (like the 20,000 schools using this tag) remains valid and will still display the translated 'Non-religious' label, and mappers can still manually enter the tag if needed."

Fixes #1939

@github-actions
Copy link
Copy Markdown

🍱 Your pull request preview is ready

Please use this preview to check your changes. Ideally use the test documentation template and document your test results by commenting on the PR. This will speed up the review process for everyone.

FYI, once this PR is merged, you can use the iD Editor Preview to test your changes in interaction with all other changes.

@matkoniecz
Copy link
Copy Markdown
Collaborator

Existing data (like the 20,000 schools using this tag) remains valid and will still display the translated 'Non-religious' label, and mappers can still manually enter the tag if needed."

are you sure? where this label and its translation are defined after this change?

@ak8abhinay
Copy link
Copy Markdown
Contributor Author

@matkoniecz,

I see now that deleting that line of code might break the label for existing data. I will update the PR to restore the 'Non-religious' string while adding an explicit options array that excludes none.

This keeps the label perfect for 20,000 schools because the translation is still defined in the strings.options object, which the editor uses to map raw tag values to human-readable text. By excluding it from the options array, we simply remove it from the 'suggested' UI buttons without deleting it from the dictionary.

This stops the button from being suggested for religious presets, while future mappers can still manually type the tag if needed, and the editor will recognize it through the restored string.

Comment thread data/fields/religion.json Outdated
Comment thread data/fields/religion.json Outdated
tyrasd
tyrasd previously requested changes Feb 18, 2026
Copy link
Copy Markdown
Member

@tyrasd tyrasd left a comment

Choose a reason for hiding this comment

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

I think it is not ideal to not be able to select the none options for features like a school that also use the religion field, but would most often be tagged as nonreligious.

See #1939 (comment) for an idea to better approach this.

@ak8abhinay
Copy link
Copy Markdown
Contributor Author

ak8abhinay commented Feb 18, 2026

@tyrasd,

This current PR matches exactly the idea you mentioned there in #1939 (comment). My thinking with the current PR was that mappers could still manually type religion=none in the tags section if they needed it like for eg. in case of schools.

The better approach I can think right now is :
Adding a 'validation warning' that only pops up when amenity=place_of_worship + religion=none. This keeps the editor easy to use for everyone while helping mappers avoid mistakes. It feels like a cleaner way to handle.

I also looked into creating separate new field file or implementing a conditional check on the none value directly within the school.json and religion.json presets. However, I’m concerned that the iD schema may not support this. Because the global field list might dominate the presets logic, even if we try to add a manual fix in a specific file the editor just ignores it and uses the main list instead.

@ak8abhinay ak8abhinay requested a review from tyrasd February 21, 2026 07:13
@matkoniecz
Copy link
Copy Markdown
Collaborator

are you sure that

and create a copy of the field that inherits the strings from the existing field, but limits the options to the values except the none option.

part was done?

@matkoniecz
Copy link
Copy Markdown
Collaborator

If you are unsure how to do this: do not worry, until few minutes ago I also was unaware.

Since then I created https://github.com/ideditor/schema-builder/pull/260/changes that may be helpful and should explain how to achieve it.

@ak8abhinay
Copy link
Copy Markdown
Contributor Author

ak8abhinay commented Mar 1, 2026

Since then I created https://github.com/ideditor/schema-builder/pull/260/changes that may be helpful and should explain how to achieve it.

@matkoniecz, thanks for the reference, this helps to understand.
So, I was copy-pasting all the strings into the religion.json itself, but I should be using "stringsCrossReference": "religion" instead in new field so it inherits the strings from the existing field. Will fix this.

@ak8abhinay
Copy link
Copy Markdown
Contributor Author

Reverted religion.json back to original, created a new religion_place_of_worship.json field that uses stringsCrossReference: "{religion}" to inherit strings from the existing field but excludes none from options, and updated place_of_worship.json to use this new field instead.

Copy link
Copy Markdown
Collaborator

@matkoniecz matkoniecz left a comment

Choose a reason for hiding this comment

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

I guess it works, though you get anyway 'no' coming from taginfo

not sure is this list good enough to disable taginfo suggestions

@matkoniecz
Copy link
Copy Markdown
Collaborator

It seems to not work as I would expect, but it looks like it is an iD bug

see openstreetmap/iD#12117

@tyrasd tyrasd dismissed their stale review April 7, 2026 09:29

addressed

@tyrasd tyrasd merged commit 03b3c48 into openstreetmap:main Apr 7, 2026
5 checks passed
@tyrasd tyrasd changed the title Remove religion=none from suggested options to prevent logical contra… Omit religion=none from suggested options of religion field for places of worship Apr 7, 2026
@tyrasd tyrasd added the field label Apr 7, 2026
@matkoniecz
Copy link
Copy Markdown
Collaborator

Thanks for the PR! It is now merged but note that before you will see it in iD few things need to happen.

iD tagging schema needs to get release (see https://github.com/openstreetmap/id-tagging-schema/releases - latest one mentioned there is just a draft). And later needs to be pulled in by iD.

Thanks again for your contribution! If you are interested in making other one it would be really welcome!

@matkoniecz
Copy link
Copy Markdown
Collaborator

openstreetmap/iD@b71105a should fix iD bug, so it has more noticeable effect once released

note: if our religion list is really complete, maybe we can and should disable showing values coming from taginfo, see https://github.com/ideditor/schema-builder?tab=readme-ov-file#autosuggestions and #1829

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

don't suggest religious=none ?

3 participants