On Wed, Feb 6, 2013 at 12:17 PM, Ralf Gommers <ralf.gommers@gmail.com> wrote:



On Wed, Feb 6, 2013 at 3:44 PM, Benjamin Root <ben.root@ou.edu> wrote:


On Tue, Feb 5, 2013 at 4:23 PM, Charles R Harris <charlesr.harris@gmail.com> wrote:
Hi All,

This post is to bring the discussion of PR #2965 to the attention of the list. There are at least three issues in play here.

1) The PR adds modes 'big' and 'thin' to the current modes 'full', 'r', 'economic' for qr factorization. The problem is that the current 'full' is actually 'thin' and 'big' should be 'full'. The solution here was to raise a FutureWarning on use of 'full', alias it to 'thin' for the time being, and at some distant time change 'full' to alias 'big'.

This is asking for problems, to gain some naming consistency. I can't tell how confusing 'full' is now, but deprecating and removing would be better than changing what it returns.

That's what the current state of the PR is, both 'full' and 'economic' are deprecated.
 
 
 
2) The 'economic' mode serves little purpose. I propose to deprecate it and add a 'qrf' mode instead, corresponding to scipy's 'raw' mode. We can't use 'raw' itself as traditionally the mode may be specified using the first letter only and that leads to a conflict with 'r'.

That's not a very good reason to not use "raw", since "raw" is a new option and you therefore don't have to apply the rule that you can give only the first letter to it.

Also the current state.
 
 

3) As suggested in 2, the use of single letter abbreviations can constrain the options in choosing mode names and they are not as informative as the full name. A possibility here is to deprecate the use of the abbreviations in favor of the full names.

I'm not feeling very strongly about this, but we have to be careful about deprecations. Possible future naming constraints on new modes is not a good reason to deprecate. This one-letter option isn't even mentioned in the docs it looks like. So why not leave that as is and ensure it keeps working (add a unit test if needed)?

Currently qr requires full names for the new modes but not for the deprecated 'full' and 'economic'. That can be changed if we use 'thin' instead of 'reduced'.

 

A longer term problem is the divergence between the numpy and scipy versions of qr. The divergence is enough that I don't see any easy way to come to a common interface, but that is something that would be desirable if possible.

This would be a problem imho. But I don't see why you can't add "raw" to numpy's qr. And if you add "big" and "thin" in numpy, you can add those modes in scipy too.

Currently I've used bfroehle's suggestions, although I'm tempted by 'thin' instead of 'reduced'

<snip>

Chuck