ropensci-archive / bomrang

:warning: ARCHIVED :warning: Australian government Bureau of Meteorology (BOM) data client for R
Other
109 stars 26 forks source link

Tests for print.bomrang_tbl() currently fail due to header values #129

Closed adamhsparks closed 3 years ago

adamhsparks commented 3 years ago

When capturing the output, more text appears in the header than in the actual printed output from the function, causing the tests to fail.

See below, the first example captures the output. The second functions as we'd expect bomrang to work not capturing the output, just printing the values to the console.

Just checking to see what the expected behaviour is here, @jonocarroll. The tests don't include the \033[36m\033[3m or \t values and did pass. Is this just my system or is it a change somewhere else?

>   x <-
+     capture.output(get_historical_weather(latlon = c(-35.2809, 149.1300),
+                                           type = "min"))

Closest station: 070351 (CANBERRA AIRPORT)

Data saved as /var/folders/hc/tft3s5bn48gb81cs99mycyf00000gn/T//RtmpjDkU9u/IDCJAC0011_070351_1800_Data.csv
> x
 [1] "\033[36m\033[3m  --- Australian Bureau of Meteorology (BOM) Data Resource ---"                     
 [2] "\033[23m\033[39m\033[36m\033[3m  (Original Request Parameters)"                                    
 [3] "\033[23m\033[39m\033[36m\033[3m  Station:\t\tCANBERRA AIRPORT [070351] "                           
 [4] "\033[23m\033[39m\033[36m\033[3m  Location:\t\tlat: -35.3088, lon: 149.2004"                        
 [5] "\033[23m\033[39m\033[36m\033[3m  Measurement / Origin:\tMin / Historical"                          
 [6] "\033[23m\033[39m\033[36m\033[3m  Timespan:\t\t2008-09-01 -- 2021-02-01 [12.5 years]"               
 [7] "\033[23m\033[39m\033[36m\033[3m  ---------------------------------------------------------------  "
 [8] "\033[23m\033[39m      product_code station_number year month day min_temperature" 

vs

> get_historical_weather(latlon = c(-35.2809, 149.1300),
+                        type = "min")

Closest station: 070351 (CANBERRA AIRPORT)

Data saved as /var/folders/hc/tft3s5bn48gb81cs99mycyf00000gn/T//RtmpjDkU9u/IDCJAC0011_070351_1800_Data.csv
  --- Australian Bureau of Meteorology (BOM) Data Resource ---
  (Original Request Parameters)
  Station:      CANBERRA AIRPORT [070351] 
  Location:     lat: -35.3088, lon: 149.2004
  Measurement / Origin: Min / Historical
  Timespan:     2008-09-01 -- 2021-02-01 [12.5 years]
  ---------------------------------------------------------------  
jonocarroll commented 3 years ago

This is expected behaviour; these are the ANSI terminal codes which control the colour of the additional information. Is there a reason to use capture.output() in the tests? Presumably because this is output prior to the result with cat().

I used {crayon} to build these

> library(crayon)
> cat(green(
+     'I am a green line ' %+%
+         blue$underline$bold('with a blue substring') %+%
+         ' that becomes green again!\n'
+ ))
I am a green line with a blue substring that becomes green again!
> green(
+     'I am a green line ' %+%
+         blue$underline$bold('with a blue substring') %+%
+         ' that becomes green again!\n'
+ )
[1] "\033[32mI am a green line \033[34m\033[4m\033[1mwith a blue substring\033[22m\033[24m\033[32m that becomes green again!\n\033[39m"

One option would be to optionally turn off the formatting via a default option, in which case you could run the tests in that mode. I can write a PR to add that if you'd like.

Alternatively (and a lot quicker) you can remove this during the tests yourself

> crayon::strip_style("\033[23m\033[39m\033[36m\033[3m  Station:\t\tCANBERRA AIRPORT [070351]")
[1] "  Station:\t\tCANBERRA AIRPORT [070351]"
adamhsparks commented 3 years ago

It’s been a while since I’ve looked at the tests. I’m tracking down why it doesn’t pass checks with CI and this popped up. This is not the reason for the delays in seeing but it did pass before. I’ll use your last suggestion to deal with it. 🙏

Sent from my iPhone

On 1 Mar 2021, at 19:36, Jonathan Carroll notifications@github.com wrote:

 This is expected behaviour; these are the ANSI terminal codes which control the colour of the additional information. Is there a reason to use capture.output() in the tests? Presumably because this is output prior to the result with cat().

I used {crayon} to build these

library(crayon) cat(green(

  • 'I am a green line ' %+%
  • blue$underline$bold('with a blue substring') %+%
  • ' that becomes green again!\n'
  • )) I am a green line with a blue substring that becomes green again! green(
  • 'I am a green line ' %+%
  • blue$underline$bold('with a blue substring') %+%
  • ' that becomes green again!\n'
  • ) [1] "\033[32mI am a green line \033[34m\033[4m\033[1mwith a blue substring\033[22m\033[24m\033[32m that becomes green again!\n\033[39m" One option would be to optionally turn off the formatting via a default option, in which case you could run the tests in that mode. I can write a PR to add that if you'd like.

Alternatively (and a lot quicker) you can remove this during the tests yourself

crayon::strip_style("\033[23m\033[39m\033[36m\033[3m Station:\t\tCANBERRA AIRPORT [070351]") [1] " Station:\t\tCANBERRA AIRPORT [070351]" — You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub, or unsubscribe.