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.

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

sage.symbolic.ring: Remove direct use of cimports from pynac #32391

Closed
mkoeppe opened this issue Aug 17, 2021 · 15 comments
Closed

sage.symbolic.ring: Remove direct use of cimports from pynac #32391

mkoeppe opened this issue Aug 17, 2021 · 15 comments

Comments

@mkoeppe
Copy link
Contributor

mkoeppe commented Aug 17, 2021

... by moving functions that need pynac functions or objects to other modules such as sage.symbolic.expression and similar refactoring.

This is preparation for #32386.

CC: @kliem @tscrim @egourgoulhon

Component: refactoring

Author: Matthias Koeppe

Branch/Commit: 7618295

Reviewer: Travis Scrimshaw

Issue created by migration from https://trac.sagemath.org/ticket/32391

@mkoeppe mkoeppe added this to the sage-9.5 milestone Aug 17, 2021
@mkoeppe

This comment has been minimized.

@mkoeppe
Copy link
Contributor Author

mkoeppe commented Aug 18, 2021

comment:2

The symbols are:

GEx, GExprSeq, GExVector, GSymbol,
g_Infinity, g_mInfinity, g_UnsignedInfinity, g_wild,
ex_to_symbol, ginac_new_symbol, ginac_symbol, is_a_symbol,
domain_complex, domain_real, domain_positive, domain_integer,
GEx_to_str_latex

@mkoeppe
Copy link
Contributor Author

mkoeppe commented Aug 18, 2021

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Aug 22, 2021

Branch pushed to git repo; I updated commit sha1. New commits:

414352csage.symbolic.ring: Just import, not cimport the new helper function _latex_Expression
5f39f2bsage.symbolic.ring.is_SymbolicVariable: Import implementation from sage.symbolic.expression
c86ad88sage.symbolic.ring.SymbolicRing._element_constructor_, _force_pyobject: Import implementation from sage.symbolic.expression
73fd88dsage.symbolic.ring.SymbolicRing.wild: Import implementation from sage.symbolic.expression
6a62fa5sage.symbolic.ring.SymbolicRing.symbol: Import implementation from sage.symbolic.expression
7618295sage.symbolic.ring.SymbolicRing._repr_element_: Import implementation from sage.symbolic.expression, remove 'cimport Expression'

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Aug 22, 2021

Commit: 7618295

@mkoeppe
Copy link
Contributor Author

mkoeppe commented Aug 22, 2021

Author: Matthias Koeppe

@tscrim
Copy link
Collaborator

tscrim commented Aug 22, 2021

Reviewer: Travis Scrimshaw

@tscrim
Copy link
Collaborator

tscrim commented Aug 22, 2021

comment:6

Green bot => positive review.

@kliem
Copy link
Contributor

kliem commented Aug 24, 2021

Changed commit from 7618295 to none

@kliem
Copy link
Contributor

kliem commented Aug 24, 2021

comment:8

Wrong ticket, sorry.


New commits:

3a954ddSymbolicRing._latex_element_: Go through new function _latex_Expression
414352csage.symbolic.ring: Just import, not cimport the new helper function _latex_Expression
5f39f2bsage.symbolic.ring.is_SymbolicVariable: Import implementation from sage.symbolic.expression
c86ad88sage.symbolic.ring.SymbolicRing._element_constructor_, _force_pyobject: Import implementation from sage.symbolic.expression
73fd88dsage.symbolic.ring.SymbolicRing.wild: Import implementation from sage.symbolic.expression
6a62fa5sage.symbolic.ring.SymbolicRing.symbol: Import implementation from sage.symbolic.expression
7618295sage.symbolic.ring.SymbolicRing._repr_element_: Import implementation from sage.symbolic.expression, remove 'cimport Expression'

@kliem
Copy link
Contributor

kliem commented Aug 24, 2021

Commit: 7618295

@mkoeppe
Copy link
Contributor Author

mkoeppe commented Aug 26, 2021

comment:9

Patchbot is green. Thanks for reviewing! (#32386 addresses a coverage issue that the patchbot plugin complains about; I hope that's OK.)

@vbraun
Copy link
Member

vbraun commented Sep 13, 2021

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants