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

Sum format returns a space for Kiloseparator #248

Closed
camponez opened this issue Jun 22, 2017 · 22 comments
Closed

Sum format returns a space for Kiloseparator #248

camponez opened this issue Jun 22, 2017 · 22 comments
Labels

Comments

@camponez
Copy link

Setup

  • MW version: 1.28.2
  • DB (MySQL etc.): 10.2.6-MariaDB-10.2.6+maria~jessie
  • PHP version:7.0.16 (fpm-fcgi)
  • SMW version: 2.5.2
  • SRF version: 2.5.0
  • Browsers and versions (if applicable): ALL

Issue

Sum format returns space for Kiloseparator.

Steps to reproduce

Use {{#ask: PARAMETER | format=sum}}

Expected result: ...

Either: with the kilo separator defined by the locale (in my case 'a dot')
Or: a standard number format (no kiloseparator and 'a dot' for decimal)

Observed result: ...

Kilo separator is a space.

@mwjames
Copy link
Contributor

mwjames commented Jun 22, 2017 via email

@camponez
Copy link
Author

camponez commented Jun 22, 2017

@mwjames
1 - I think it should be . for pt-br
2 - I don't see that line on the SW I have installed. (edit: Oh, I see now the commit is 3 days old)
3 - Adding the line didn't change much... should I do anything else?

@camponez
Copy link
Author

When using table format all numbers are correctly formatted.

@mwjames
Copy link
Contributor

mwjames commented Jun 22, 2017 via email

@camponez
Copy link
Author

I would expect (given the previously outlined rules of comma and space) that a result for the number 3000.2 (ISO format) is displayed both in sum and table format (for user language pt-br) as:
3 000,2

That's exactly the issue. It only shows space for kiloseparator when I use sum format. For table it works correctly.

Sum format: 3 000,02
Table format: 3.000,02

@camponez
Copy link
Author

Here it works: https://is.gd/bK22hl
Here it doesn't: https://is.gd/YOoXJs

@mwjames
Copy link
Contributor

mwjames commented Jun 22, 2017 via email

@camponez
Copy link
Author

camponez commented Jun 22, 2017

It depends on the changes to the SMW-core [0] (available with 2.5.3) where space is defined as smw_kiloseparator for language pt-br and would make both to be displayed

I have altered my code adding the sw_kiloseparator line just like the patch, but using . instead. It didn't change anything. Is there any other change that need to be done?

@mwjames
Copy link
Contributor

mwjames commented Jun 22, 2017 via email

@mwjames
Copy link
Contributor

mwjames commented Jun 22, 2017 via email

@camponez
Copy link
Author

I will try that. Thank you.

@kghbln
Copy link
Member

kghbln commented Jun 22, 2017

I actually have no longer any idea what the expected kilo separator for "pt-br" is. If a space is wrong it should be changed here. I am backing out of this.

@kghbln kghbln added the bug label Jun 22, 2017
@jaideraf
Copy link
Member

Not sure if the question is already clear...

In Portuguese (and Brazilian Portuguese), decimal is used with a comma [1].
So, "1.5" in English is presented as "1,5" in Portuguese.

"Portuguese uses a dot to separate thousands, eg. 2.367.900 = 2,367,900" [2]

Here it doesn't: https://is.gd/YOoXJs

This shoud be presented as "336.397,5" or, better yet, "336.397,50" because it is a property for currency.

[1] https://en.wikipedia.org/wiki/Decimal_mark#Hindu.E2.80.93Arabic_numeral_system
[2] http://www.omniglot.com/language/numbers/portuguese.htm

@kghbln
Copy link
Member

kghbln commented Jun 22, 2017

The examples on wikipedia show a space so I added a "space". @jaideraf Please do me the favour and adjust at translatewiki.net as it fits the "pt-br" needs.

@mwjames
Copy link
Contributor

mwjames commented Jun 22, 2017 via email

@jaideraf
Copy link
Member

@kghbln
Copy link
Member

kghbln commented Jun 22, 2017

@jaideraf Thanks a lot. One already can have a hard time to figure this out for "de-ch" so "pt" and friends are even more difficult for non-natives. These changes will roll in next Tuesday and I will backport for fluff.

@camponez
Copy link
Author

If we wanted to make them behave equally then those lines would need to be replaced with:

if ( $outputformat != "-" ) { 
- global $wgLang; 
- $number = $wgLang->formatNum( $number ); 
+ $dataValue = \SMW\DataValueFactory::getInstance()->newDataValueByType( '_num' ); 
+ $number = $dataValue->getLocalizedFormattedNumber( $number ); 
}

Yes. That worked for me

@jaideraf
Copy link
Member

... and would require some integration tests...

@mwjames I am trying to understand that JSON test thing to see if I can help a little, but I couldn't make it work. Why, at the wiki, the sum value is 30 030 025,75 but, in the test result, the sum value is 31 517 575? If it's not too wrong, maybe you could adjust this test to make it to work.

{
	"description": "Test for sum result format in pt-br lang",
	"setup": [
		{
			"page": "MediaWiki:Smw_kiloseparator/pt-br",
			"namespace": "NS_MEDIAWIKI",
			"contents": "."
		},
		{
			"page": "MediaWiki:Smw_decseparator/pt-br",
			"namespace": "NS_MEDIAWIKI",
			"contents": ","
		},
		{
			"page": "Property:Has_number",
			"namespace": "SMW_NS_PROPERTY",
			"contents": "[[Has type::Number]]"
		},
		{
			"page": "Has_number",
			"namespace":"NS_MAIN",
			"contents": "[[Has number::30000000]] [[Has number::15000]] [[Has number::15000,25]] [[Has number::25,50]] Sum: {{#show:{{PAGENAME}}|?Has number|format=sum}}"
		}
	],
	"tests": [
		{
			"type": "parser",
			"about": "#0 result format for sum in pt-br lang (test for kilo and decimal separators)",
			"subject": "Has_number",
			"assert-output": {
				"to-contain": [
					"30.030.025,75"
				]
			}
		}
	],
	"settings": {
		"wgContLang": "pt-br",
		"wgLang": "pt-br",
		"smwgNamespacesWithSemanticLinks": {
			"NS_MAIN": true,
			"SMW_NS_PROPERTY": true
		}
	},
	"meta": {
		"version": "2",
		"is-incomplete": false,
		"debug": false
	}
}

@mwjames
Copy link
Contributor

mwjames commented Jun 24, 2017

I am trying to understand that JSON test thing to see if I can help a little, but I couldn't make it work. Why, at the wiki, the sum value is 30 030 025,75 but, in the test result, the sum value is 31 517 575? If it's not too wrong, maybe you could adjust this test to make it to work.

Thanks, I added [0] to clarify possible interferences when designing tests. As for the example, I'll wait until the translation arrives, then the modified test should pass.

We are not relying on "page": "MediaWiki:Smw_kiloseparator/pt-br", because MW's message system reads from the JSON files and I have found no good method to fetch it from the DB (or page) during testing therefore we have to wait until the translation from tnw appear.

{
	"description": "Test for math/sum result format in pt-br lang",
	"setup": [
		{
			"page": "Has number",
			"namespace": "SMW_NS_PROPERTY",
			"contents": "[[Has type::Number]]"
		},
		{
			"page": "Example/Math/1",
			"contents": "[[Has number::30000000]] [[Has number::15000]] [[Has number::15000,25]] [[Has number::25,50]] "
		},
		{
			"page": "Example/Math/Q.1",
			"contents": "{{#show: Example/Math/1 |?Has number|format=sum }}"
		},
		{
			"page": "Example/Math/Q.2",
			"contents": "{{#show: Example/Math/1 |?Has number|format=max }}"
		},
		{
			"page": "Example/Math/Q.3",
			"contents": "{{#show: Example/Math/1 |?Has number|format=min }}"
		},
		{
			"page": "Example/Math/Q.4",
			"contents": "{{#show: Example/Math/1 |?Has number|format=product }}"
		},
		{
			"page": "Example/Math/Q.5",
			"contents": "{{#show: Example/Math/1 |?Has number|format=average }}"
		},
		{
			"page": "Example/Math/Q.6",
			"contents": "{{#show: Example/Math/1 |?Has number|format=median }}"
		}
	],
	"tests": [
		{
			"type": "parser",
			"about": "#0 format=sum (kilo and decimal separators)",
			"subject": "Example/Math/Q.1",
			"assert-output": {
				"to-contain": [
					"30.030.025,75"
				]
			}
		},
		{
			"type": "parser",
			"about": "#1 format=max (kilo and decimal separators)",
			"subject": "Example/Math/Q.2",
			"assert-output": {
				"to-contain": [
					"30.000.000"
				]
			}
		},
		{
			"type": "parser",
			"about": "#2 format=min (kilo and decimal separators)",
			"subject": "Example/Math/Q.3",
			"assert-output": {
				"to-contain": [
					"25,5"
				]
			}
		},
		{
			"type": "parser",
			"about": "#3 format=product (kilo and decimal separators)",
			"subject": "Example/Math/Q.4",
			"assert-output": {
				"to-contain": [
					"1,721279e+17"
				]
			}
		},
		{
			"type": "parser",
			"about": "#4 format=average (kilo and decimal separators)",
			"subject": "Example/Math/Q.5",
			"assert-output": {
				"to-contain": [
					"7.507.506,438"
				]
			}
		},
		{
			"type": "parser",
			"about": "#5 format=median (kilo and decimal separators)",
			"subject": "Example/Math/Q.6",
			"assert-output": {
				"to-contain": [
					"15.000,125"
				]
			}
		}
	],
	"settings": {
		"wgContLang": "pt-br",
		"wgLang": "pt-br",
		"smwgNamespacesWithSemanticLinks": {
			"NS_MAIN": true,
			"SMW_NS_PROPERTY": true
		}
	},
	"meta": {
		"version": "2",
		"is-incomplete": false,
		"debug": false
	}
}

[0] SemanticMediaWiki/SemanticMediaWiki@d0f8494

@jaideraf
Copy link
Member

Oh, much better, thanks. 👏

@mwjames
Copy link
Contributor

mwjames commented Jul 8, 2017

Addressed with #250.

@mwjames mwjames closed this as completed Jul 8, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

4 participants