Opened 2 years ago

Closed 2 years ago

#23779 closed defect (fixed)

dev_tools is wrongly using globals()

Reported by: vdelecroix Owned by:
Priority: major Milestone: sage-8.1
Component: misc Keywords: thursdaysbdx
Cc: chapoton Merged in:
Authors: Vincent Delecroix Reviewers: Sébastien Labbé
Report Upstream: N/A Work issues:
Branch: 6716719 (Commits) Commit: 6716719f225fe4ce20c2129db1d7972aaff58ddd
Dependencies: Stopgaps:

Description (last modified by vdelecroix)

The built-in function globals() is restricted to the module where it is defined. From Python doc

globals()

    Return a dictionary representing the current global symbol
    table. This is always the dictionary of the current module
    (inside a function or method, this is the module where it
     is defined, not the module from which it is called).

Its usage in dev_tools.find_objects_from_name is just completely wrong as it should check for the sage global namespace. Moreover, such check in globals would better be done in import_statements directly.

As reported in #19444 there is the following misbehavior

sage: import_statements('log')
# **Warning**: distinct objects with name 'log' in:
#   - sage.functions.log
#   - math
#   - sage.functions
#   - sage.misc.functional
from sage.misc.functional import log

that is corrected into

sage: import_statements('log')
from sage.functions.log import log

Change History (6)

comment:1 Changed 2 years ago by vdelecroix

  • Branch set to u/vdelecroix/23779
  • Commit set to 87329a2d280a0def9dd9fb479fa764ee1bd1a0cf
  • Status changed from new to needs_review

New commits:

87329a223779: fix sage namespace lookup in import_statements

comment:2 Changed 2 years ago by vdelecroix

  • Description modified (diff)

comment:3 Changed 2 years ago by slabbe

  • Branch changed from u/vdelecroix/23779 to u/slabbe/23779
  • Commit changed from 87329a2d280a0def9dd9fb479fa764ee1bd1a0cf to 6716719f225fe4ce20c2129db1d7972aaff58ddd

I postive review your commit.

I added a commit to update the doc of the function find_objects_from_name.

Note that I rebased your commit on top of 8.1.beta5.

Feel free to change the status to positive review if you agree with my changes.


New commits:

83c8c4e23779: fix sage namespace lookup in import_statements
671671923779: updated doc of find_objects_from_name
Last edited 2 years ago by slabbe (previous) (diff)

comment:4 Changed 2 years ago by slabbe

  • Keywords thursdaysbdx added
  • Reviewers set to Sébastien Labbé

comment:5 Changed 2 years ago by vdelecroix

  • Status changed from needs_review to positive_review

Good! Thanks.

comment:6 Changed 2 years ago by vbraun

  • Branch changed from u/slabbe/23779 to 6716719f225fe4ce20c2129db1d7972aaff58ddd
  • Resolution set to fixed
  • Status changed from positive_review to closed
Note: See TracTickets for help on using tickets.