Issue #252 fix can't subset

[AndreMikulec]

Fix of
Can't subset empty xts by date anymore #252
yields the desired result . . .

> library(xts)
> testxts=xts(1,as.Date('2018-07-18'))
> str(testxts['2017'][as.Date('2018-07-17')])
An 'xts' object of zero-width

> devtools::session_info()
Session info -------------------------------------------------------------------------------------------------------
 setting  value                       
 version  R version 3.5.1 (2018-07-02)
 system   x86_64, mingw32             
 ui       RStudio (1.1.453)           
 language (EN)                        
 collate  English_United States.1252  
 tz       America/Chicago             
 date     2018-07-23                  

Packages -----------------------------------------------------------------------------------------------------------
 package    * version date       source                          
 base       * 3.5.1   2018-07-02 local                           
 commonmark   1.5     2018-04-28 CRAN (R 3.5.1)                  
 compiler     3.5.1   2018-07-02 local                           
 datasets   * 3.5.1   2018-07-02 local                           
 devtools     1.13.6  2018-06-27 CRAN (R 3.5.1)                  
 digest       0.6.15  2018-01-28 CRAN (R 3.5.0)                  
 graphics   * 3.5.1   2018-07-02 local                           
 grDevices  * 3.5.1   2018-07-02 local                           
 grid         3.5.1   2018-07-02 local                           
 lattice      0.20-35 2017-03-25 CRAN (R 3.5.0)                  
 magrittr   * 1.5     2014-11-22 CRAN (R 3.5.0)                  
 memoise      1.1.0   2017-04-21 CRAN (R 3.5.0)                  
 methods    * 3.5.1   2018-07-02 local                           
 R6           2.2.2   2017-06-17 CRAN (R 3.5.0)                  
 Rcpp         0.12.18 2018-07-23 CRAN (R 3.5.1)                  
 roxygen2     6.0.1   2017-02-06 CRAN (R 3.5.1)                  
 rstudioapi   0.7     2017-09-07 CRAN (R 3.5.0)                  
 stats      * 3.5.1   2018-07-02 local                           
 stringi      1.2.4   2018-07-20 CRAN (R 3.5.1)                  
 stringr      1.3.1   2018-05-10 CRAN (R 3.5.0)                  
 tools        3.5.1   2018-07-02 local                           
 utils      * 3.5.1   2018-07-02 local                           
 withr        2.1.2   2018-04-27 Github (jimhester/withr@79d7b0d)
 xml2         1.2.0   2018-01-24 CRAN (R 3.5.0)                  
 xts        * 0.11-0  <NA>       local                           
 yaml         2.1.19  2018-05-01 CRAN (R 3.5.0)                  
 zoo        * 1.8-3   2018-07-16 CRAN (R 3.5.1)      

[joshuaulrich]

Thank you for your patch and PR. I'm closing this without merging because I chose to fix the issue in a different way. In the future, please note the pull request section of the contributing guide. Specifically the point on commit messages and adding unit tests. The commit message should make it clear to others why the change is necessary, and the unit test should ensure that the change isn't accidentally reverted in the future.

[AndreMikulec]

I did eventually read CONTRIBUTING.md

I did not see the "xts/.github/CONTRIBUTING.md" until I had already written the 'pull request'. Github only presents the CONTRIBUTING when a 'pull request' is about to happen ( and this event seems late. )

At the point that I saw CONTRIBUTING that first time, I backed out of my 'pull request' and
'modified some' of my 'pull request,' but I had already modified some of it too much.

Perhaps, also putting "Contributing" near the top of the README.md
( and also keeping it at the bottom ) of the README.md may help in seeing the "Contributing."

Maybe I misunderstood the 'commit' instructions.

The commit message should make it clear to others why the change is necessary

from
1d707c5

Fix failure of test..xts_class

Reinstate the behaviour as of v0.10-2.  .indexCLASS should take
precedence over tclass.

Fixes #249.

the case seems that the commit message is

Fix failure of test..xts_class

I think that Github breaks this down into ...

commit title and a commit description ( words: "Add an optional extended description..." )

Maybe this explains what I need to do next time

How to commit a change with both “message” and “description” from the command line? [duplicate]
https://github.com/AndreMikulec/expressions/edit/master/Givens_Siegel_Faber_Johnson.R

Later, I did see the "Commit Messages" section in CONTRIBUTING.md, but at that time, the situation was too late.

I did 'think about' unit tests for a moment.
I looked at this

xts/inst/unitTests/Makefile

that has this line here that I can not 'easily' repeat (because I am on Windows)

R=/Users/jryan/Downloads/R-2.10.1/bin/R

so, I determined that I could not easily integrate a unit test, so I did not look deeper into unit tests.
Maybe some more cross-platform portable version of "R=/Users/jryan/Downloads/R-2.10.1/bin/R" may be useful.

xts is a great package. The maintainers and contributors are wonderful people.

[joshuaulrich]

@AndreMikulec Thank you for taking time to explain your perspective and provide constructive feedback!

There may be a way to make the Contributing guide appear sooner in the pull request (PR) process via templates. I will need to investigate and experiment.

You are correct that there are 2 components to a commit message: 1) the title (~50 characters), and 2) the body (where you explain 'why'). This is native to Git, and not specific to GitHub or any other host (e.g. GitLab, Bitbucket, etc).

Thanks for the feedback about running the unit tests. I agree that is something I need to address.