Skip to content

Conversation

tornaria
Copy link
Contributor

@tornaria tornaria commented Jan 3, 2024

  1. fix test for cython functions in save_session()
    This fixes save_session() broken when cython changes #37003
  2. fix handling of lazy import variables in save_session()
    This fixes save_session() is broken when interactive #37002
  3. remove unused extra_globals attribute in class DocTestTask
    This was introduced for the transition from python 2 to 3, no longer used
  4. don't sort the output of sage.misc.session.show_identifiers()
    There's no need to sort here... sorting was introduced in baedc41 in order to fix doctest errors, but dictionaries now keep the insertion order. Thus show_identifiers() will return the variables in the order they were introduced for the first time which seems reasonable and it's fine to test.

📝 Checklist

  • The title is concise, informative, and self-explanatory.
  • The description explains in detail what this PR is about.
  • I have linked a relevant issue or discussion.
  • I have created tests covering the changes.

Fixes: #37002, #37003

This was introduced to help transition from python 2 to python 3 and is
no longer used.
There's no need to do it. Moreover, dictionaries in python keep
insertion order, so we don't need to sort for testing either.

Note that in the test

         sage: a = 10
         sage: factor = 20
         sage: show_identifiers()
         ['factor', 'a']

factor is indeed inserted first at sagemath startup (as a function)
Copy link

github-actions bot commented Jan 3, 2024

Documentation preview for this PR (built with commit 2673c8b; changes) is ready! 🎉

Copy link
Contributor

@fchapoton fchapoton left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

looks good

vbraun pushed a commit to vbraun/sage that referenced this pull request Jan 16, 2024
sagemathgh-37004: Fixes in save_session()
    
1. fix test for cython functions in save_session()
   This fixes sagemath#37003
2. fix handling of lazy import variables in save_session()
   This fixes sagemath#37002
3. remove unused `extra_globals` attribute in class DocTestTask
   This was introduced for the transition from python 2 to 3, no longer
used
4. don't sort the output of `sage.misc.session.show_identifiers()`
   There's no need to sort here... sorting was introduced in baedc41
in order to fix doctest errors, but dictionaries now keep the insertion
order. Thus `show_identifiers()` will return the variables in the order
they were introduced for the first time which seems reasonable and it's
fine to test.

### 📝 Checklist

- [x] The title is concise, informative, and self-explanatory.
- [x] The description explains in detail what this PR is about.
- [x] I have linked a relevant issue or discussion.
- [x] I have created tests covering the changes.

Fixes: sagemath#37002, sagemath#37003
    
URL: sagemath#37004
Reported by: Gonzalo Tornaría
Reviewer(s): Frédéric Chapoton
vbraun pushed a commit to vbraun/sage that referenced this pull request Jan 16, 2024
sagemathgh-37004: Fixes in save_session()
    
1. fix test for cython functions in save_session()
   This fixes sagemath#37003
2. fix handling of lazy import variables in save_session()
   This fixes sagemath#37002
3. remove unused `extra_globals` attribute in class DocTestTask
   This was introduced for the transition from python 2 to 3, no longer
used
4. don't sort the output of `sage.misc.session.show_identifiers()`
   There's no need to sort here... sorting was introduced in baedc41
in order to fix doctest errors, but dictionaries now keep the insertion
order. Thus `show_identifiers()` will return the variables in the order
they were introduced for the first time which seems reasonable and it's
fine to test.

### 📝 Checklist

- [x] The title is concise, informative, and self-explanatory.
- [x] The description explains in detail what this PR is about.
- [x] I have linked a relevant issue or discussion.
- [x] I have created tests covering the changes.

Fixes: sagemath#37002, sagemath#37003
    
URL: sagemath#37004
Reported by: Gonzalo Tornaría
Reviewer(s): Frédéric Chapoton
vbraun pushed a commit to vbraun/sage that referenced this pull request Jan 16, 2024
sagemathgh-37004: Fixes in save_session()
    
1. fix test for cython functions in save_session()
   This fixes sagemath#37003
2. fix handling of lazy import variables in save_session()
   This fixes sagemath#37002
3. remove unused `extra_globals` attribute in class DocTestTask
   This was introduced for the transition from python 2 to 3, no longer
used
4. don't sort the output of `sage.misc.session.show_identifiers()`
   There's no need to sort here... sorting was introduced in baedc41
in order to fix doctest errors, but dictionaries now keep the insertion
order. Thus `show_identifiers()` will return the variables in the order
they were introduced for the first time which seems reasonable and it's
fine to test.

### 📝 Checklist

- [x] The title is concise, informative, and self-explanatory.
- [x] The description explains in detail what this PR is about.
- [x] I have linked a relevant issue or discussion.
- [x] I have created tests covering the changes.

Fixes: sagemath#37002, sagemath#37003
    
URL: sagemath#37004
Reported by: Gonzalo Tornaría
Reviewer(s): Frédéric Chapoton
@vbraun vbraun merged commit 00b9047 into sagemath:develop Jan 22, 2024
@tornaria tornaria deleted the save_session branch January 22, 2024 20:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

save_session() broken when cython changes save_session() is broken when interactive
4 participants