Opened 8 years ago

Closed 7 years ago

#15138 closed defect (fixed)

BuiltinFunction._is_registered is giving false negatives

Reported by: ohanar Owned by:
Priority: major Milestone: sage-6.2
Component: symbolics Keywords:
Cc: Merged in:
Authors: R. Andrew Ohana Reviewers: Ralf Stephan
Report Upstream: N/A Work issues:
Branch: 4c04c34 (Commits, GitHub, GitLab) Commit: 4c04c344e42858e0722b19bc9cc83a265b1a9a61
Dependencies: Stopgaps:

Status badges

Description

This is easily encountered by the following:

sage: loads(dumps(sin)) == sin # derives from GinacFunction
True
sage: loads(dumps(cot)) == cot # should also be True
False

Change History (6)

comment:1 Changed 8 years ago by ohanar

  • Cc burcin added

comment:2 Changed 8 years ago by ohanar

  • Cc burcin removed

The following seems to fix the issue:

@@ -853,18 +853,12 @@ cdef class BuiltinFunction(Function):
 
         # if match, get operator from function table
         global sfunction_serial_dict
-        if serial != -1 and sfunction_serial_dict.has_key(self._name) and \
-                sfunction_serial_dict[self._name].__class__ == self.__class__:
+        if serial != -1 and sfunction_serial_dict.has_key(serial) and \
+                sfunction_serial_dict[serial].__class__ == self.__class__:
                     # if the returned function is of the same type
                     self._serial = serial
                     return True
 
-        # search the function table to check if any of this type
-        for key, val in sfunction_serial_dict.iteritems():
-            if key == self._name and val.__class__ == self.__class__:
-                self._serial = key
-                return True
-
         return False
 
     def __reduce__(self):

I don't know the code, so I don't know what other implications this change might have (I didn't run the test suite).

comment:3 Changed 7 years ago by vbraun_spam

  • Milestone changed from sage-6.1 to sage-6.2

comment:4 Changed 7 years ago by rws

  • Status changed from new to needs_review

comment:5 Changed 7 years ago by rws

  • Authors set to R. Andrew Ohana
  • Branch set to u/rws/ticket/15138
  • Commit set to 4c04c344e42858e0722b19bc9cc83a265b1a9a61
  • Reviewers set to Ralf Stephan
  • Status changed from needs_review to positive_review

Cannot see any problems, long tests fine in symbolic. I adapted the patch to newer code, added a doctest for this.


New commits:

4c04c34Trac #15138: reviewer's patch: adapt to new code, add doctest

comment:6 Changed 7 years ago by vbraun

  • Branch changed from u/rws/ticket/15138 to 4c04c344e42858e0722b19bc9cc83a265b1a9a61
  • Resolution set to fixed
  • Status changed from positive_review to closed
Note: See TracTickets for help on using tickets.