[Rd] proposal for use ICU for timezone convertion on windows and a draft patch
Dear All, Last week I found an issue about R Sys.timezone function on Windows and send email to this list. This week , I read quite a few docs and codes about windows time zone to IANA time zone conversion. and found in unicode.org's ICU library has a function ucal_getTimeZoneIDForWindowsID which do the time zone convert. Microsoft also use it for time zone convertion in dotnet 6 and 7.(https://github.com/dotnet/runtime/blob/main/src/native/libs/System.Globalization.Native/pal_timeZoneInfo.c) . IMHO, use this function to implement time zone convert on windows is better than R's hand-write conversion implementation, use the ICU , it will update the conversion when unicode org upgrade the ICU library and no more hand-write conversion code needed. Also RTools already included the ICU library,and R on windows build link ICU lib on default , so no additional library or tools need for use this function for time zone conversion. so I wrote a quick and dirty patch use the ucal_getTimeZoneIDForWindowsID, I think maybe better discuss here before I send patch to bugs.r-project.org. I put the patch on https://github.com/armgong/misc-r-patch/blob/main/registryTZ-ICU.diff. this patch mainly do following three steps : 1 read current English time zone name from registry ; 2 read current region name from registry(this also affect conversion); 3 use ucal_getTimeZoneIDForWindowsID implement the conversion. please feel free to share your thoughts or feedbacks of this patch. if it doable ,then I will modify it and send it to bugs.r-project.org best regards, Yu Gong [[alternative HTML version deleted]] __ R-devel@r-project.org mailing list https://stat.ethz.ch/mailman/listinfo/r-devel
Re: [Rd] rhub vs. CRAN fedora-*-devel, using armadillo & slapack
Thank you all, for these advices.
So I try to fix OMP_THREADS, cleanup tests, and display explicitly what test is
running by moving in tests/ instead of tests/testthat/...
Next step should be to investigate blocking test using a reporter (maybe
"list").
For now, waiting for CRAN results...
Yann
-Message d'origine-
De : Duncan Murdoch
Envoyé : mercredi 11 janvier 2023 00:36
À : Sebastian Meyer ; Ivan Krylov ;
RICHET Yann
Cc : Pascal Havé ; R-devel@r-project.org
Objet : Re: [Rd] rhub vs. CRAN fedora-*-devel, using armadillo & slapack
On 10/01/2023 4:07 p.m., Sebastian Meyer wrote:
> Am 10.01.23 um 21:28 schrieb Duncan Murdoch:
>> On 10/01/2023 2:05 p.m., Ivan Krylov wrote:
>>> On Tue, 10 Jan 2023 16:27:53 +
>>> RICHET Yann wrote:
>>>
In facts, 10 threads are asked by armadillo for some LinAlg, which
backs to two threads as warned.
>>>
>>> I think you're right about your tests de-facto using two threads,
>>> but it might be a good idea to _default_ to up to two threads in
>>> tests and examples. This is especially valuable for third-party
>>> developers who have to mass-test packages (one of which could be
>>> rlibkriging) in parallel.
>>>
- is there any reason that could explain that fedora-*-devel is so
slow for this package or compilation of Rcpp/testthat ?
>>>
>>> Compilation time is definitely not the reason. Something in tests/*
>>> actually runs for 30 minutes by itself.
>>>
- is there any chance that I can get a deeper log of what happened ?
>>>
>>> If you split your tests into separate files under tests/*.R instead
>>> of using a single tests/testthat.R calling the rest of the tests, R
>>> will be able to show you the individual test file that hung and
>>> maybe the line where it happened. (Also, you'll get per-file
>>> timing.) But that is potentially a huge investment: you may have to
>>> rewrite the tests to work outside the testthat harness, and you'd
>>> also have to prepare another CRAN submission just to have those
>>> tests run. It's also against CRAN policy to knowingly submit a package with
>>> unfixed ERRORs.
>>>
>>> (Currently, R can only tell you that the tests hung in the
>>> test_check('rlibkriging') call in the tests/testthat.R, which isn't
>>> precise enough.)
>>
>> You can specify a different "reporter" in the test_check() call, and
>> it will print more useful information. I don't think there's a
>> perfect one, but
>>
>> test_check('rlibkriging', reporter = "progress")
>>
>> should at least show you the tests that finished running before the
>> timeout.
>
> I had similar problems with testthat and timeouts when mass-checking
> packages on patched R versions. My notes say
>
>> ## testthat's 'LocationReporter' does cat() after each expectation ##
>> so we can see results even if timeout is reached
>> options(testthat.default_check_reporter = c("Location", "Check"))
>
> The help("LocationReporter") says: "This reporter simply prints the
> location of every expectation and error. This is useful if you're
> trying to figure out the source of a segfault, or you want to figure
> out which code triggers a C/C++ breakpoint"
>
> HTH!
Yes, that looks like it would pin down the location of the problem.
Here's some sample output from it:
Running ‘testthat.R’ [41s/42s]
Running the tests in ‘tests/testthat.R’ failed.
Last 13 lines of output:
Start test: can use constructed calls in verify_output() (#945)
'test-verify-output.R:55' [success]
End test: can use constructed calls in verify_output() (#945)
Start test: verify_output() doesn't use cli unicode by default
'test-verify-output.R:65' [success]
'test-verify-output.R:73' [success]
End test: verify_output() doesn't use cli unicode by default
Start test: verify_output() handles carriage return
'test-verify-output.R:82' [success]
End test: verify_output() handles carriage return
Error: Test failures
Execution halted
One other thing: you enabled this by using
options(testthat.default_check_reporter = c("Location", "Check"))
before running the tests; the package writer could do the same thing by using
test_check('rlibkriging', reporter = c("Location", "Check"))
Duncan Murdoch
__
R-devel@r-project.org mailing list
https://stat.ethz.ch/mailman/listinfo/r-devel
Re: [Rd] rhub vs. CRAN fedora-*-devel, using armadillo & slapack
On 11/01/2023 12:35 p.m., RICHET Yann wrote:
Thank you all, for these advices.
So I try to fix OMP_THREADS, cleanup tests, and display explicitly what test is
running by moving in tests/ instead of tests/testthat/...
Next step should be to investigate blocking test using a reporter (maybe
"list").
For now, waiting for CRAN results...
I think Sebastian or my suggestion is easier than redoing all of your
tests. They are each one line changes.
Duncan Murdoch
Yann
-Message d'origine-
De : Duncan Murdoch
Envoyé : mercredi 11 janvier 2023 00:36
À : Sebastian Meyer ; Ivan Krylov ; RICHET
Yann
Cc : Pascal Havé ; R-devel@r-project.org
Objet : Re: [Rd] rhub vs. CRAN fedora-*-devel, using armadillo & slapack
On 10/01/2023 4:07 p.m., Sebastian Meyer wrote:
Am 10.01.23 um 21:28 schrieb Duncan Murdoch:
On 10/01/2023 2:05 p.m., Ivan Krylov wrote:
On Tue, 10 Jan 2023 16:27:53 +
RICHET Yann wrote:
In facts, 10 threads are asked by armadillo for some LinAlg, which
backs to two threads as warned.
I think you're right about your tests de-facto using two threads,
but it might be a good idea to _default_ to up to two threads in
tests and examples. This is especially valuable for third-party
developers who have to mass-test packages (one of which could be
rlibkriging) in parallel.
- is there any reason that could explain that fedora-*-devel is so
slow for this package or compilation of Rcpp/testthat ?
Compilation time is definitely not the reason. Something in tests/*
actually runs for 30 minutes by itself.
- is there any chance that I can get a deeper log of what happened ?
If you split your tests into separate files under tests/*.R instead
of using a single tests/testthat.R calling the rest of the tests, R
will be able to show you the individual test file that hung and
maybe the line where it happened. (Also, you'll get per-file
timing.) But that is potentially a huge investment: you may have to
rewrite the tests to work outside the testthat harness, and you'd
also have to prepare another CRAN submission just to have those
tests run. It's also against CRAN policy to knowingly submit a package with
unfixed ERRORs.
(Currently, R can only tell you that the tests hung in the
test_check('rlibkriging') call in the tests/testthat.R, which isn't
precise enough.)
You can specify a different "reporter" in the test_check() call, and
it will print more useful information. I don't think there's a
perfect one, but
test_check('rlibkriging', reporter = "progress")
should at least show you the tests that finished running before the
timeout.
I had similar problems with testthat and timeouts when mass-checking
packages on patched R versions. My notes say
## testthat's 'LocationReporter' does cat() after each expectation ##
so we can see results even if timeout is reached
options(testthat.default_check_reporter = c("Location", "Check"))
The help("LocationReporter") says: "This reporter simply prints the
location of every expectation and error. This is useful if you're
trying to figure out the source of a segfault, or you want to figure
out which code triggers a C/C++ breakpoint"
HTH!
Yes, that looks like it would pin down the location of the problem.
Here's some sample output from it:
Running ‘testthat.R’ [41s/42s]
Running the tests in ‘tests/testthat.R’ failed.
Last 13 lines of output:
Start test: can use constructed calls in verify_output() (#945)
'test-verify-output.R:55' [success]
End test: can use constructed calls in verify_output() (#945)
Start test: verify_output() doesn't use cli unicode by default
'test-verify-output.R:65' [success]
'test-verify-output.R:73' [success]
End test: verify_output() doesn't use cli unicode by default
Start test: verify_output() handles carriage return
'test-verify-output.R:82' [success]
End test: verify_output() handles carriage return
Error: Test failures
Execution halted
One other thing: you enabled this by using
options(testthat.default_check_reporter = c("Location", "Check"))
before running the tests; the package writer could do the same thing by using
test_check('rlibkriging', reporter = c("Location", "Check"))
Duncan Murdoch
__
R-devel@r-project.org mailing list
https://stat.ethz.ch/mailman/listinfo/r-devel
Re: [Rd] rhub vs. CRAN fedora-*-devel, using armadillo & slapack
On Wed, 11 Jan 2023 13:09:25 -0500 Duncan Murdoch wrote: > I think Sebastian or my suggestion is easier than redoing all of your > tests. They are each one line changes. Yes, sorry about that. Definitely try a verbose reporter first instead of redoing the tests. I'll remember not to give this advice next time. -- Best regards, Ivan __ R-devel@r-project.org mailing list https://stat.ethz.ch/mailman/listinfo/r-devel
Re: [Rd] rhub vs. CRAN fedora-*-devel, using armadillo & slapack
On 11 January 2023 at 17:35, RICHET Yann wrote: | Thank you all, for these advices. | | So I try to fix OMP_THREADS, cleanup tests, and display explicitly what test is running by moving in tests/ instead of tests/testthat/... | Next step should be to investigate blocking test using a reporter (maybe "list"). | For now, waiting for CRAN results... BTW your package failed its tests for me in a reverse-depends run when I had `ccache` as usual in the definition of CC, CXX, FC, CXX14, ... That usually works with other packages using CMake. Maybe something you could look into. Dirk -- dirk.eddelbuettel.com | @eddelbuettel | e...@debian.org __ R-devel@r-project.org mailing list https://stat.ethz.ch/mailman/listinfo/r-devel
Re: [Rd] proposal for use ICU for timezone convertion on windows and a draft patch
still work on it. this morning , I found the better and short sloution of
this, the ucal_getDefaultTimeZone will return IANA formart zone name on windows.
so the main code for this issue listing here:
/* Longest currently is 31 chars */
static char Olson[64] = "";
/* use ucal_getDefaultTimeZone to get IANA default time zone name */
const char *getTZinfo(void)
{
UChar defaultzone[64];
UErrorCode status_default = U_ZERO_ERROR;
int32_t i = ucal_getDefaultTimeZone(defaultzone, ARRAYSIZE(defaultzone),
&status_default);
if (U_SUCCESS(status_default)) {
wcstombs(Olson, defaultzone, 64);
Olson[63] = '\0';
#ifdef DEBUG
printf("TZ = %s\n", Olson);
#endif
return Olson;
}
else{
return "unknown";
}
}
I still maintain a longer version of this patch, but it should not needed if
ucal_getDefaultTimeZone is reliable .
the longer version when ucal_getDefaultTimeZone fail to got IANA time zone
name , it do extra step to got time zone name( 1 read current English time
zone name from registry ; 2 read current region name from registry(this also
affect conversion); 3 use ucal_getTimeZoneIDForWindowsID implement the
conversion.)
please feel free to share your thoughts or feedbacks of this short version
patch.
best regards,
Yu Gong
From: R-devel on behalf of yu gong
Sent: Wednesday, January 11, 2023 22:25
To: r-devel@r-project.org
Subject: [Rd] proposal for use ICU for timezone convertion on windows and a
draft patch
Dear All,
Last week I found an issue about R Sys.timezone function on Windows and send
email to this list. This week , I read quite a few docs and codes about
windows time zone to IANA time zone conversion. and found in unicode.org's ICU
library has a function ucal_getTimeZoneIDForWindowsID which do the time zone
convert. Microsoft also use it for time zone convertion in dotnet 6 and
7.(https://github.com/dotnet/runtime/blob/main/src/native/libs/System.Globalization.Native/pal_timeZoneInfo.c)
.
IMHO, use this function to implement time zone convert on windows is better
than R's hand-write conversion implementation, use the ICU , it will update the
conversion when unicode org upgrade the ICU library and no more hand-write
conversion code needed. Also RTools already included the ICU library,and R on
windows build link ICU lib on default , so no additional library or tools need
for use this function for time zone conversion.
so I wrote a quick and dirty patch use the ucal_getTimeZoneIDForWindowsID, I
think maybe better discuss here before I send patch to bugs.r-project.org.
I put the patch on
https://github.com/armgong/misc-r-patch/blob/main/registryTZ-ICU.diff.
this patch mainly do following three steps : 1 read current English time zone
name from registry ; 2 read current region name from registry(this also affect
conversion); 3 use ucal_getTimeZoneIDForWindowsID implement the conversion.
please feel free to share your thoughts or feedbacks of this patch. if it
doable ,then I will modify it and send it to bugs.r-project.org
best regards,
Yu Gong
[[alternative HTML version deleted]]
__
R-devel@r-project.org mailing list
https://stat.ethz.ch/mailman/listinfo/r-devel
[[alternative HTML version deleted]]
__
R-devel@r-project.org mailing list
https://stat.ethz.ch/mailman/listinfo/r-devel
Re: [Rd] rhub vs. CRAN fedora-*-devel, using armadillo & slapack
Well, I tried to move the tests outside testthat.R logic, because I expect that
CRAN output will not give me the reporter results... and as I re-submitted the
package, I wanted to ensure readable result. But maybe I am wrong about that...
?
-Message d'origine-
De : Duncan Murdoch
Envoyé : mercredi 11 janvier 2023 19:09
À : RICHET Yann ; Sebastian Meyer ; Ivan
Krylov
Cc : Pascal Havé ; R-devel@r-project.org
Objet : Re: [Rd] rhub vs. CRAN fedora-*-devel, using armadillo & slapack
On 11/01/2023 12:35 p.m., RICHET Yann wrote:
> Thank you all, for these advices.
>
> So I try to fix OMP_THREADS, cleanup tests, and display explicitly what test
> is running by moving in tests/ instead of tests/testthat/...
> Next step should be to investigate blocking test using a reporter (maybe
> "list").
> For now, waiting for CRAN results...
I think Sebastian or my suggestion is easier than redoing all of your tests.
They are each one line changes.
Duncan Murdoch
>
> Yann
>
> -Message d'origine-
> De : Duncan Murdoch Envoyé : mercredi 11
> janvier 2023 00:36 À : Sebastian Meyer ; Ivan Krylov
> ; RICHET Yann Cc : Pascal
> Havé ; R-devel@r-project.org Objet : Re: [Rd]
> rhub vs. CRAN fedora-*-devel, using armadillo & slapack
>
> On 10/01/2023 4:07 p.m., Sebastian Meyer wrote:
>> Am 10.01.23 um 21:28 schrieb Duncan Murdoch:
>>> On 10/01/2023 2:05 p.m., Ivan Krylov wrote:
On Tue, 10 Jan 2023 16:27:53 +
RICHET Yann wrote:
> In facts, 10 threads are asked by armadillo for some LinAlg, which
> backs to two threads as warned.
I think you're right about your tests de-facto using two threads,
but it might be a good idea to _default_ to up to two threads in
tests and examples. This is especially valuable for third-party
developers who have to mass-test packages (one of which could be
rlibkriging) in parallel.
> - is there any reason that could explain that fedora-*-devel is so
> slow for this package or compilation of Rcpp/testthat ?
Compilation time is definitely not the reason. Something in tests/*
actually runs for 30 minutes by itself.
> - is there any chance that I can get a deeper log of what happened ?
If you split your tests into separate files under tests/*.R instead
of using a single tests/testthat.R calling the rest of the tests, R
will be able to show you the individual test file that hung and
maybe the line where it happened. (Also, you'll get per-file
timing.) But that is potentially a huge investment: you may have to
rewrite the tests to work outside the testthat harness, and you'd
also have to prepare another CRAN submission just to have those
tests run. It's also against CRAN policy to knowingly submit a package
with unfixed ERRORs.
(Currently, R can only tell you that the tests hung in the
test_check('rlibkriging') call in the tests/testthat.R, which isn't
precise enough.)
>>>
>>> You can specify a different "reporter" in the test_check() call, and
>>> it will print more useful information. I don't think there's a
>>> perfect one, but
>>>
>>> test_check('rlibkriging', reporter = "progress")
>>>
>>> should at least show you the tests that finished running before the
>>> timeout.
>>
>> I had similar problems with testthat and timeouts when mass-checking
>> packages on patched R versions. My notes say
>>
>>> ## testthat's 'LocationReporter' does cat() after each expectation
>>> ## so we can see results even if timeout is reached
>>> options(testthat.default_check_reporter = c("Location", "Check"))
>>
>> The help("LocationReporter") says: "This reporter simply prints the
>> location of every expectation and error. This is useful if you're
>> trying to figure out the source of a segfault, or you want to figure
>> out which code triggers a C/C++ breakpoint"
>>
>> HTH!
>
> Yes, that looks like it would pin down the location of the problem.
> Here's some sample output from it:
>
> Running ‘testthat.R’ [41s/42s]
> Running the tests in ‘tests/testthat.R’ failed.
> Last 13 lines of output:
> Start test: can use constructed calls in verify_output() (#945)
> 'test-verify-output.R:55' [success]
> End test: can use constructed calls in verify_output() (#945)
>
> Start test: verify_output() doesn't use cli unicode by default
> 'test-verify-output.R:65' [success]
> 'test-verify-output.R:73' [success]
> End test: verify_output() doesn't use cli unicode by default
>
> Start test: verify_output() handles carriage return
> 'test-verify-output.R:82' [success]
> End test: verify_output() handles carriage return
>
> Error: Test failures
> Execution halted
>
> One other thing: you enabled this by using
>
> options(testthat.default_check_reporter = c("Location", "Check"))
>
> before running the tests; the package writer could do the same thi
