Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Some preparations for binding=True #25848

Closed
jdemeyer opened this issue Jul 12, 2018 · 10 comments
Closed

Some preparations for binding=True #25848

jdemeyer opened this issue Jul 12, 2018 · 10 comments
Labels
c: cython p: major / 3 t: enhancement
Milestone
sage-8.4

Comments

@jdemeyer
Copy link

These are a few assorted fixes to enable binding=True:

  1. Enable binding=True on some non-performance-critical unpickling functions. These regularly occur in __reduce__ doctests, causing annoying doctest failures. In a few cases, the unpickling function can be removed completely.

  2. Compile __getmetaclass__ with binding=False otherwise it won't work.

  3. Resolve a cyclic import involving sage.rings.convert.mpfi by explicitly importing it.

  4. In src/sage/categories/category.py, assign the helpers as staticmethod to prevent binding.

Component: cython

Author: Jeroen Demeyer

Branch/Commit: e240df3

Reviewer: Travis Scrimshaw

Issue created by migration from https://trac.sagemath.org/ticket/25848
@jdemeyer jdemeyer added this to the sage-8.4 milestone Jul 12, 2018
@jdemeyer

This comment has been minimized.

Sign in to view
@jdemeyer
Copy link
Author

Branch: u/jdemeyer/some_preparations_for_binding_true

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Jul 13, 2018

Branch pushed to git repo; I updated commit sha1. This was a forced push. New commits:

6e6e1ebSome preparations for binding=True

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Jul 13, 2018

Commit: 6e6e1eb

@tscrim
Copy link
Collaborator

tscrim commented Jul 13, 2018

Reviewer: Travis Scrimshaw

@tscrim
Copy link
Collaborator

tscrim commented Jul 13, 2018

comment:5

Green bot => positive review.

@tscrim
Copy link
Collaborator

tscrim commented Jul 17, 2018

comment:6

Looks like one real failure from the patchbot:

sage -t --long src/sage/modular/modsym/p1list.pyx  # 2 doctests failed

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Jul 19, 2018

Changed commit from 6e6e1eb to e240df3

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Jul 19, 2018

Branch pushed to git repo; I updated commit sha1. This was a forced push. New commits:

e240df3Some preparations for binding=True

@vbraun
Copy link
Member

vbraun commented Aug 5, 2018

Changed branch from u/jdemeyer/some_preparations_for_binding_true to e240df3

@vbraun vbraun closed this as completed in 9740141 Aug 5, 2018
@jdemeyer jdemeyer mentioned this issue Nov 20, 2021
Closed
mkoeppe added a commit to mkoeppe/sage that referenced this issue Sep 21, 2023
@mkoeppe
src/sage/rings/number_field/number_field_element.pyx: Remove unpickli…
bf78977 
…ng functions deprecated in sagemath#25848 (2018)
vbraun pushed a commit to vbraun/sage that referenced this issue Sep 23, 2023
sagemathgh-36307: sage.rings: Remove code deprecated in sagemath#23204
9ea5127 
, sagemath#24483, sagemath#24371, sagemath#24511, sagemath#25848, sagemath#26105, sagemath#28481, sagemath#29010, sagemath#29412, sagemath#30332, sagemath#30372, sagemath#31345, sagemath#32375, sagemath#32606, sagemath#32610, sagemath#32612, sagemath#32641, sagemath#32660, sagemath#32750, sagemath#32869, sagemath#33602

    
<!-- ^^^^^
Please provide a concise, informative and self-explanatory title.
Don't put issue numbers in there, do this in the PR body below.
For example, instead of "Fixes sagemath#1234" use "Introduce new method to
calculate 1+1"
-->
<!-- Describe your changes here in detail -->

<!-- Why is this change required? What problem does it solve? -->
<!-- If this PR resolves an open issue, please link to it here. For
example "Fixes sagemath#12345". -->
<!-- If your change requires a documentation PR, please link it
appropriately. -->

### 📝 Checklist

<!-- Put an `x` in all the boxes that apply. -->
<!-- If your change requires a documentation PR, please link it
appropriately -->
<!-- If you're unsure about any of these, don't hesitate to ask. We're
here to help! -->
<!-- Feel free to remove irrelevant items. -->

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

### ⌛ Dependencies

<!-- List all open PRs that this PR logically depends on
- sagemath#12345: short description why this is a dependency
- sagemath#34567: ...
-->

<!-- If you're unsure about any of these, don't hesitate to ask. We're
here to help! -->
    
URL: sagemath#36307
Reported by: Matthias Köppe
Reviewer(s):
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
c: cython p: major / 3 t: enhancement
Projects
None yet
Milestone
sage-8.4
Development

No branches or pull requests
3 participants
@vbraun @tscrim @jdemeyer