descriptive warning when using the eq overloaded operator on non-nume…

…ric strings Instead of saying "isn't numeric in null operation", provide a more descriptive warning that indicates the value and the operator being used. See <http://sourceforge.net/p/pdl/bugs/332>, <#33> for more information.
PDLPorters · Mar 7, 2015 · 2d25c52 · 2d25c52
1 parent b4d6f04
commit 2d25c52
Show file tree

Hide file tree

Showing 2 changed files with 70 additions and 1 deletion.
diff --git a/Basic/Core/Core.pm b/Basic/Core/Core.pm
@@ -719,6 +719,20 @@ sub topdl {PDL->topdl(@_)}
 
 ####################### Overloaded operators #######################
 
+# This is to used warn if an operand is non-numeric or non-PDL.
+sub warn_non_numeric_op_wrapper {
+	my ($cb, $op_name) = @_;
+	return sub {
+		my $op1 = shift;
+		my $op2 = shift;
+		unless( Scalar::Util::looks_like_number($op2)
+			|| ( Scalar::Util::blessed($op2) && $op2->isa('PDL') )
+			) {
+			warn "'$op2' is not numeric nor a PDL in operator $op_name";
+		}
+	}
+}
+
 { package PDL;
   # use UNIVERSAL 'isa'; # need that later in info function
   use Carp;
@@ -739,7 +753,8 @@ sub topdl {PDL->topdl(@_)}
 		"<="    => \&PDL::le,       # in1, in2, swap if true
 		">="    => \&PDL::ge,       # in1, in2, swap if true
 		"=="    => \&PDL::eq,       # in1, in2
-		"eq"    => \&PDL::eq,       # in1, in2
+		"eq"    => PDL::Core::warn_non_numeric_op_wrapper(\&PDL::eq, 'eq'),
+		                            # in1, in2
 		"!="    => \&PDL::ne,       # in1, in2
 
 		"<<"    => \&PDL::shiftleft,  # in1, in2, swap if true

diff --git a/t/issue-sf-332.t b/t/issue-sf-332.t
@@ -0,0 +1,54 @@
+use Test::More tests => 5;
+use Test::Warn;
+
+use strict;
+use warnings;
+
+## Issue information
+##
+## Name: "isn't numeric in null operation" warning could be more helpful
+##
+## <http://sourceforge.net/p/pdl/bugs/332/>
+## <https://github.com/PDLPorters/pdl/issues/33>
+
+use PDL::Config;
+use PDL::LiteF;
+
+# The following code calls the PDL::Ops::eq() function via the operator
+# overload for the eq operator. Because the Perl eq operator is usually used
+# for strings, the default warning of "isn't numeric in null operation" is
+# confusing. Comparing a PDL against a string should give a more useful
+# warning.
+
+my $numeric_warning = [qr/not numeric nor a PDL/];
+my $no_warning = undef;
+
+sub check_eq_warnings {
+	my ($string, $warning) = @_;
+	warning_like { (pdl() eq $string); } $warning;
+	warning_like { ($string eq pdl()); } $warning;
+}
+
+subtest "String 'x' is not numeric and should warn" => sub {
+	check_eq_warnings('x', $numeric_warning);
+};
+subtest "String 'nancy' is not numeric and should warn" => sub {
+	check_eq_warnings('nancy', $numeric_warning);
+};
+subtest "String 'inf' is numeric" => sub {
+	check_eq_warnings('inf', $no_warning);
+};
+subtest "String 'nan' is numeric" => sub {
+	check_eq_warnings('nan', $no_warning);
+};
+TODO: {
+	# implementing this might require checking for strings that can be made into PDLs
+	local $TODO = "Using the eq operator with the string 'bad' might be a good feature";
+	todo_skip "Bad values disabled", 1 unless $PDL::Config{WITH_BADVAL};
+
+	subtest "String 'bad' is numeric (in PDL)" => sub {
+		check_eq_warnings('bad', $no_warning);
+	};
+}
+
+done_testing;