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

mapcss [functionresult() <= functionresult()] gives bad results with numbers #1882

Closed
Famlam opened this issue Jun 2, 2023 · 3 comments · Fixed by #1886
Closed

mapcss [functionresult() <= functionresult()] gives bad results with numbers #1882

Famlam opened this issue Jun 2, 2023 · 3 comments · Fixed by #1886
Labels

Comments

@Famlam
Copy link
Collaborator

Famlam commented Jun 2, 2023

way 680531945 has the following tags

building:levels=11 
building:min_level=9
building:part=yes

Having the following mapcss selector from combinations.mapcss in JOSM (where you can temporarily replace area by way until that PR has been merged)

area[/^(building\|building:part)$/][height =~ /^[0-9]+(\.[0-9]+)?( m)?$/][min_height =~ /^[0-9]+(\.[0-9]+)?( m)?$/][get(split(" ", tag(height)), 0) <= get(split(" ", tag(min_height)), 0)],
area[/^(building\|building:part)$/][building:levels][building:min_level][tag("building:levels") <= tag("building:min_level")] {
throwWarning: tr("{0} is lower or equal to {1} on {2}", "{1.key}", "{2.key}", "{0.key}");
group: tr("suspicious tag combination");
}

JOSM will not warn.
Osmose will show the following message:

building:levels is lower or equal to building:min_level on building:part

So apparently JOSM does a numeric comparison (11 > 9), while Osmose does a string ("11" < "9") comparison for [tag("building:levels") <= tag("building:min_level")]

Similar for way 680531956 with height = 104 and min_height = 96.

Maybe we'll have to do some checks with "value".isdecimal() or similar in the redefinition of <= (etcetera) in mapcss_lib.py.

@Famlam Famlam added the bug label Jun 2, 2023
@Famlam
Copy link
Collaborator Author

Famlam commented Jun 2, 2023

It seems that the following mapcss passes all tests in JOSM:

node[a][b][tag("a") > tag("b")] {
  throwError: tr("{0}-{1}", "{0.tag}", "{1.tag}");
  assertNoMatch: "node a=3  b=6";
  assertNoMatch: "node a=X  b=Y";
  assertNoMatch: "node a=3a b=6a";
  assertNoMatch: "node a=3a b=6";
  assertNoMatch: "node a=3  b=6a";
  assertMatch:   "node b=3  a=6";
  assertMatch:   "node b=3  a=12"; /* Fails in Osmose */
  assertNoMatch: "node b=X  a=Y"; /* Fails in Osmose */
  assertNoMatch: "node b=3a a=6a"; /* Fails in Osmose */
  assertNoMatch: "node b=3a a=6"; /* Fails in Osmose */
  assertNoMatch: "node b=3  a=6a"; /* Fails in Osmose */
}

node[a > 2] {
  throwError: tr("{0}", "{0.tag}");
  assertMatch: "node a=3";
  assertNoMatch: "node a=X";
  assertNoMatch: "node a=1";
}

Which seems to imply that the way to interpret >, <, >=, <= in mapcss context is to just try to convert both left and right operands to numbers, and if it fails, just abort. (I.e. never compare strings).

(Technically it's also what the wiki says: "Comparison for numeric values.")

This is already implemented as such for selectors like [a > 2], but when the right hand side value isn't a number it currently falls back to string comparison.

@frodrigo
Copy link
Member

frodrigo commented Jun 3, 2023

Ok. It should not be hard to change that.

@Famlam
Copy link
Collaborator Author

Famlam commented Jun 3, 2023

Agree, it's on my planning for tonight :)
(Unless you're faster ;) )

Famlam added a commit to Famlam/osmose-backend that referenced this issue Jun 3, 2023
See osm-fr#1882 :
always use numerical comparison for `<` / `>` / `<=` / `>=`
frodrigo pushed a commit that referenced this issue Jun 3, 2023
See #1882 :
always use numerical comparison for `<` / `>` / `<=` / `>=`
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 a pull request may close this issue.

2 participants