Combined name for basque language #2484

Crashillo · 2025-04-01T08:40:25Z

Due to the so long names presents in the Basque country, both basque and spanish ways, is quite common they're shorted through the common parts.

This PR introduces a function to merge two names, as another valid form for naming inside the sp_eu rule

For instance:

name=Calle Severo Ochoa kalea
name:eu=Severo Ochoa kalea
name:es=Calle Severo Ochoa

Crashillo · 2025-04-01T11:03:19Z

I don't know what's wrong with the linter :(

Famlam · 2025-04-01T11:22:12Z

The linter is unrelated, I'm testing a fix but you can ignore that for now
Can you add a test case (at the bottom of the file)?

Crashillo · 2025-04-01T15:39:14Z

My expertise in python is quite limited. AFAIK, the assertion should be true or false, not None... I guess I'm missing something

Famlam · 2025-04-01T18:06:02Z

The test functions (node, way and relation; in this case they're equal as they refer to each other) return a list containing errors if an error is found, or None or False if no error is found. So if you have to check for an error (=not adhering to the rules), use assert self.p.way(...), and if you need to check that no error is thrown, use assert not self.p.way(...)

assert doesn't need to receive a real True or False, just something truthy or falsey :)

frodrigo · 2025-04-01T18:15:29Z

Thank you for the PR submission. But first. This way of doing, it is a good practice attested by the community ? There is wiki page of forum about this ?

Crashillo · 2025-04-01T19:40:29Z

The test functions (node, way and relation; in this case they're equal as they refer to each other) return a list containing errors if an error is found, or None or False if no error is found. So if you have to check for an error (=not adhering to the rules), use assert self.p.way(...), and if you need to check that no error is thrown, use assert not self.p.way(...)

assert doesn't need to receive a real True or False, just something truthy or falsey :)

I do not see what's the problem in this clause versus the rest of assertions I added

assert self.p.way(None, {"name": u"Vicente Blasco Ibañez kalea / Calle Vicente Blasco Ibáñez", "name:es": u"Calle Vicente Blasco Ibáñez", "name:eu": u"Vicente Blasco Ibañez kalea"}, None)
E       AssertionError: assert None
E        +  where None = <bound method Name_Multilingual.way of <plugins.Name_Multilingual.Name_Multilingual object at 0x7f22ccabf490>>(None, {'name': 'Vicente Blasco Ibañez kalea / Calle Vicente Blasco Ibáñez', 'name:es': 'Calle Vicente Blasco Ibáñez', 'name:eu': 'Vicente Blasco Ibañez kalea'}, None)
E        +    where <bound method Name_Multilingual.way of <plugins.Name_Multilingual.Name_Multilingual object at 0x7f22ccabf490>> = <plugins.Name_Multilingual.Name_Multilingual object at 0x7f22ccabf490>.way
E        +      where <plugins.Name_Multilingual.Name_Multilingual object at 0x7f22ccabf490> = <plugins.Name_Multilingual.Test testMethod=test_eu>.p

osmose-backend/plugins/Name_Multilingual.py

Lines 368 to 378 in 6e17766

    
           assert self.p.way(None, {"name": u"Carretera Ollaretxe errepidea", "name:es": u"Carretera Ollaretxe", "name:eu": u"Ollaretxe errepidea"}, None) 
        
           assert self.p.way(None, {"name": u"Kale Nagusia / Calle Mayor", "name:es": u"Calle Nagusia", "name:eu": u"Kale Nagusia"}, None) 
        
           assert self.p.way(None, {"name": u"Vicente Blasco Ibañez kalea / Calle Vicente Blasco Ibáñez", "name:es": u"Calle Vicente Blasco Ibáñez", "name:eu": u"Vicente Blasco Ibañez kalea"}, None) 
        
           assert self.p.way(None, {"name": u"Calle San Diego kalea", "name:es": u"Calle San Diego", "name:eu": u"San Diego kalea"}, None) 
        
           assert self.p.way(None, {"name": u"Calle Islas Canarias / Kanariar Uharteen kalea", "name:es": u"Calle Islas Canarias", "name:eu": u"Kanariar Uharteen kalea"}, None) 
        
           assert not self.p.way(None, {"name": u"Calle Islas Canarias / Kanariar Uharteen kalea", "name:es": u"Calle Canarias", "name:eu": u"Kanarias kalea"}, None) 
        
           assert not self.p.way(None, {"name": u"Calle San Diego", "name:es": u"", "name:eu": u"San Diego kalea"}, None) 
        
           assert not self.p.way(None, {"name": u"Vicente Blasco Ibañez kalea / Calle Vicente Blasco Ibáñez", "name:es": u"", "name:eu": u""}, None) 
        
           assert not self.p.way(None, {"name": u"Kale Nagusia", "name:es": u"Calle Nagusia", "name:eu": u""}, None) 
        
           assert not self.p.way(None, {"name": u"Carretera Ollaretxe", "name:es": u"Carretera Ollaretxe", "name:eu": u"Ollaretxe errepidea"}, None)

Crashillo · 2025-04-01T20:11:55Z

it is a good practice attested by the community ? There is wiki page of forum about this ?

Basque community is not such, but standalone mappers. Due to the language (and the recent history) you know the naming is somehow conflictive. Besides, basque names plus spanish names generates a very long names (example, and also note the renaming the users do after normalization).

This combined way is very usual in the basque country, much simpler than having <name:es> / <name:eu>, what usually leads to confusion, even to the tourists. I will update such detail in the spanish wiki; however, basque mappers tend to ignore it. My purpose doing this PR is about to reduce (and to allow) a common way of naming things.

By far, osmose name-multilingual error is one of the most repeated issues:

plugins/Name_Multilingual.py

Famlam · 2025-04-11T21:50:48Z

Regarding the failing test:

assert self.p.way(None, {"name": u"Vicente Blasco Ibañez kalea / Calle Vicente Blasco Ibáñez", "name:es": u"Calle Vicente Blasco Ibáñez", "name:eu": u"Vicente Blasco Ibañez kalea"}, None)

means: check that there IS an issue with this combination of tags. (The analyser returns None at least something Falsely if there's nothing to complain about)

However, the following piece of code:

[
    {"name": tags["name:"+lang[0]].strip()},
    {"name": tags["name:"+lang[1]].strip()},
    {"name": tags["name:"+lang[0]].strip() + separator + tags["name:"+lang[1]].strip()},
    {"name": tags["name:"+lang[1]].strip() + separator + tags["name:"+lang[0]].strip()},
    {"name": self.merge_sp_eu(tags["name:"+lang[0]], tags["name:"+lang[1]]).strip()}
]

gives the following result on this input (for valid name values):

[
    {'name': 'Calle Vicente Blasco Ibáñez'},
    {'name': 'Vicente Blasco Ibañez kalea'},
    {'name': 'Calle Vicente Blasco Ibáñez / Vicente Blasco Ibañez kalea'},
    {'name': 'Vicente Blasco Ibañez kalea / Calle Vicente Blasco Ibáñez'},
    {'name': ''}
]

As you can see, the 4th option equals the name tag, thus considering it a valid result, returning no error

p.s.: I'm a bit worried the '' will end up as a fix suggestion, by head I thought it was only stripped if it was None. But we'll see that later.
p.p.s.: note that the original lint issue was fixed but the remaining one is due to this PR

Crashillo · 2025-04-12T13:58:02Z

I was misunderstanding how the assert works, I thought the opposite what you just mentioned. Now I had to redo the full aggregator function for clarity, I think now is quite straightforward

Crashillo · 2025-04-27T18:00:51Z

Any reason to keep it open?

Famlam · 2025-05-05T15:20:22Z

Any reason to keep it open?

I haven't had the time to review it yet, busy times. Sorry for the wait

plugins/Name_Multilingual.py

frodrigo · 2025-05-15T20:29:35Z

plugins/Name_Multilingual.py

+        assert self.p.way(None, {"name": "Calle San Diego", "name:es": "", "name:eu": "San Diego kalea"}, None)
+        assert self.p.way(None, {"name": "Kale Nagusia", "name:es": "Calle Mayor", "name:eu": ""}, None)


Suggested change

assert self.p.way(None, {"name": "Calle San Diego", "name:es": "", "name:eu": "San Diego kalea"}, None)

assert self.p.way(None, {"name": "Kale Nagusia", "name:es": "Calle Mayor", "name:eu": ""}, None)

assert self.p.way(None, {"name": "Calle San Diego"}, None)

assert self.p.way(None, {"name:eu": "San Diego kalea"}, None)

assert self.p.way(None, {"name:es": "Calle Mayor"}, None)

assert self.p.way(None, {"name": "Calle San Diego", "name:eu": "San Diego kalea"}, None)

assert self.p.way(None, {"name": "Calle San Diego", "name:es": "", "name:eu": "San Diego kalea"}, None)

assert self.p.way(None, {"name": "Kale Nagusia", "name:es": "Calle Mayor"}, None)

assert self.p.way(None, {"name": "Kale Nagusia", "name:es": "Calle Mayor", "name:eu": ""}, None)

I'd say the failing test come from this change. As I understand, it's not the same, since what I was testing in the first clause was the presence of name and name:eu, with different values, and not name:es. And similar for the second one: name and name:es different, and no name:eu

No sure to understand your response. The function should not fails.

Look at the failing job: it fails just for these changes you made. Undo these changes and letś try again.

AFAI understand you're not testing the same case I did, aren't you? I wanted to test the cases I before-commented: two tags, name and name:lang, different values each other. It should warn the user "hey, something is missing here"

I remove when there is only name.

The code should not crash, even if there is nothing to be done.

plugins/Name_Multilingual.py

frodrigo · 2025-05-15T20:47:35Z

I add few tests, than fails. Please can you review my changes.

plugins/Name_Multilingual.py

frodrigo · 2025-05-18T14:19:59Z

@Crashillo I think, there is a misunderstanding here. The tests need to pass for only one name variant. Actually it crash. The plugin could not crash, it stops the whole check for the "country". I cannot merge a plugin that crash.

See. I run it on real data, and as I expected, it crash.

time ./osmose_run.py --country=spain_euskadi_vizcaya --no-clean --analyser sax --plugin=Name_Multilingual
[...]
2025-05-18 14:14:46   Analysing file /data/work/fred/extracts/spain_euskadi_vizcaya.osm.pbf
[INFO] Reading the file/data/work/fred/extracts/spain_euskadi_vizcaya.osm.pbf
2025-05-18 14:14:48   error: Fail on <bound method Name_Multilingual.node of <plugins.Name_Multilingual.Name_Multilingual object at 0x7f9dda96a8e0>> with {'id': 4379264289, 'lon': -2.9604194000000033, 'lat': 43.274954500000035, 'tag': {'amenity': 'pharmacy', 'healthcare': 'pharmacy', 'name:eu': 'Botika'}, 'timestamp': 1670352603}, {'amenity': 'pharmacy', 'healthcare': 'pharmacy', 'name:eu': 'Botika'}
2025-05-18 14:14:48   Closing reader and parser
2025-05-18 14:14:48   error: error on analyse sax...
2025-05-18 14:14:48     Traceback (most recent call last):
2025-05-18 14:14:48       File "/home/fred/osmose-backend/./osmose_run.py", line 275, in execc
2025-05-18 14:14:48         analyser_obj.analyser()
2025-05-18 14:14:48       File "/home/fred/osmose-backend/analysers/analyser_sax.py", line 69, in analyser
2025-05-18 14:14:48         self._run_analyse()
2025-05-18 14:14:48       File "/home/fred/osmose-backend/analysers/analyser_sax.py", line 491, in _run_analyse
2025-05-18 14:14:48         self.parser.CopyTo(self)
2025-05-18 14:14:48       File "/home/fred/osmose-backend/modules/OsmPbf_libosmbf.py", line 85, in CopyTo
2025-05-18 14:14:48         osm_pbf_parser.read_osm_pbf(self._pbf_file, self)
2025-05-18 14:14:48       File "/home/fred/osmose-backend/modules/OsmPbf_libosmbf.py", line 98, in node
2025-05-18 14:14:48         self._output.NodeCreate(data)
2025-05-18 14:14:48       File "/home/fred/osmose-backend/analysers/analyser_sax.py", line 183, in NodeCreate
2025-05-18 14:14:48         res = meth(data, tags)
2025-05-18 14:14:48       File "/home/fred/osmose-backend/plugins/Name_Multilingual.py", line 171, in node
2025-05-18 14:14:48         a = self.aggregator(tags)
2025-05-18 14:14:48       File "/home/fred/osmose-backend/plugins/Name_Multilingual.py", line 75, in aggregator
2025-05-18 14:14:48         if str1 and name != str1 and separator not in name:
2025-05-18 14:14:48     TypeError: argument of type 'NoneType' is not iterable

Crashillo · 2025-05-18T15:05:23Z

The tests need to pass for only one name variant

I don't understand what do you mean with this. The goal of the PR is about allowing the combined form as a valid result.

Look at the following table: it has a more readable view of the test cases. I hope you understand what's the purpose of this PR:

name	name:es	name:eu	isValid	comment
Carretera Ollaretxe errepidea	Carretera Ollaretxe	Ollaretxe errepidea	✅	New combined form
Kale Nagusia / Calle Mayor	Calle Mayor	Kale Nagusia	✅	Correct form already exists (Basque first, Spanish second)
Vicente Blasco Ibañez kalea / Calle Vicente Blasco Ibáñez	Calle Vicente Blasco Ibáñez	Vicente Blasco Ibañez kalea	✅	Correct form already exists (Basque first, Spanish second)
Calle San Diego kalea	Calle San Diego	San Diego kalea	✅	New combined form
Calle Islas Canarias / Kanariar Uharteen kalea	Calle Islas Canarias	Kanariar Uharteen kalea	✅	Correct form already exists (Spanish first, Basque second)
Vicente Blasco Ibañez kalea / Calle Vicente Blasco Ibáñez	(empty)	(empty)	✅	Only name with separator, it's valid
Calle Islas Canarias / Kanariar Uharteen kalea	Calle Canarias	Kanarias kalea	❌	name is different from both Basque and Spanish
Calle San Diego	(empty)	San Diego kalea	❌	name and name:eu differ, Spanish is missing and name should be combined
Kale Nagusia	Calle Mayor	(empty)	❌	name and name:es differ, Basque is missing and name should be combined
Carretera Ollaretxe	Carretera Ollaretxe	Ollaretxe errepidea	❌	Should be correct as a new combined form

Crashillo added 3 commits April 1, 2025 10:27

Update Name_Multilingual.py

1022b78

fix typo

82c0115

use self for the helper

edd7507

Crashillo added 2 commits April 1, 2025 16:42

Add tests

43620f7

fix typo

6e17766

frodrigo reviewed Apr 5, 2025

View reviewed changes

plugins/Name_Multilingual.py Outdated Show resolved Hide resolved

Crashillo added 2 commits April 10, 2025 09:46

Fix typo

99a05f4

Redo function

63969cc

fix logic for testing

ce1ed22

Crashillo added 2 commits April 12, 2025 16:19

Merge branch 'osm-fr:dev' into patch-1

9e2c125

fix lint

ce89f73

frodrigo reviewed May 15, 2025

View reviewed changes

plugins/Name_Multilingual.py Outdated Show resolved Hide resolved

Update plugins/Name_Multilingual.py

367e967

frodrigo reviewed May 15, 2025

View reviewed changes

plugins/Name_Multilingual.py Outdated Show resolved Hide resolved

Update plugins/Name_Multilingual.py

b0b8463

frodrigo reviewed May 15, 2025

View reviewed changes

Update plugins/Name_Multilingual.py

3b4a206

frodrigo reviewed May 15, 2025

View reviewed changes

plugins/Name_Multilingual.py Outdated Show resolved Hide resolved

Update plugins/Name_Multilingual.py

efadd4c

frodrigo reviewed May 15, 2025

View reviewed changes

plugins/Name_Multilingual.py Outdated Show resolved Hide resolved

Update plugins/Name_Multilingual.py

616a708

frodrigo reviewed May 17, 2025

View reviewed changes

plugins/Name_Multilingual.py Outdated Show resolved Hide resolved

frodrigo and others added 2 commits May 17, 2025 16:55

Update plugins/Name_Multilingual.py

cc3aee3

fix tests

d43a8c2

Crashillo requested a review from frodrigo May 18, 2025 12:42

		assert self.p.way(None, {"name": "Calle San Diego", "name:es": "", "name:eu": "San Diego kalea"}, None)
		assert self.p.way(None, {"name": "Kale Nagusia", "name:es": "Calle Mayor", "name:eu": ""}, None)

-        assert self.p.way(None, {"name": "Calle San Diego", "name:es": "", "name:eu": "San Diego kalea"}, None)
-        assert self.p.way(None, {"name": "Kale Nagusia", "name:es": "Calle Mayor", "name:eu": ""}, None)
+        assert self.p.way(None, {"name": "Calle San Diego"}, None)
+        assert self.p.way(None, {"name:eu": "San Diego kalea"}, None)
+        assert self.p.way(None, {"name:es": "Calle Mayor"}, None)
+        assert self.p.way(None, {"name": "Calle San Diego", "name:eu": "San Diego kalea"}, None)
+        assert self.p.way(None, {"name": "Calle San Diego", "name:es": "", "name:eu": "San Diego kalea"}, None)
+        assert self.p.way(None, {"name": "Kale Nagusia", "name:es": "Calle Mayor"}, None)
+        assert self.p.way(None, {"name": "Kale Nagusia", "name:es": "Calle Mayor", "name:eu": ""}, None)

Combined name for basque language #2484

Are you sure you want to change the base?

Combined name for basque language #2484

Uh oh!

Conversation

Crashillo commented Apr 1, 2025

Uh oh!

Crashillo commented Apr 1, 2025

Uh oh!

Famlam commented Apr 1, 2025

Uh oh!

Crashillo commented Apr 1, 2025

Uh oh!

Famlam commented Apr 1, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

frodrigo commented Apr 1, 2025

Uh oh!

Crashillo commented Apr 1, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Crashillo commented Apr 1, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Famlam commented Apr 11, 2025

Uh oh!

Crashillo commented Apr 12, 2025

Uh oh!

Crashillo commented Apr 27, 2025

Uh oh!

Famlam commented May 5, 2025

Uh oh!

Uh oh!

Uh oh!

frodrigo May 15, 2025

Choose a reason for hiding this comment

Uh oh!

Crashillo May 16, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

frodrigo May 16, 2025

Choose a reason for hiding this comment

Uh oh!

Crashillo May 16, 2025

Choose a reason for hiding this comment

Uh oh!

frodrigo May 17, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

frodrigo commented May 15, 2025

Uh oh!

Uh oh!

frodrigo commented May 18, 2025

Uh oh!

Crashillo commented May 18, 2025

Uh oh!

Uh oh!

Famlam commented Apr 1, 2025 •

edited

Loading

Crashillo commented Apr 1, 2025 •

edited

Loading

Crashillo commented Apr 1, 2025 •

edited

Loading

Crashillo May 16, 2025 •

edited

Loading