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.functions.log.log: Move to sage.misc.functional #32794

Closed
mkoeppe opened this issue Oct 29, 2021 · 17 comments
Closed

sage.functions.log.log: Move to sage.misc.functional #32794

mkoeppe opened this issue Oct 29, 2021 · 17 comments

Comments

@mkoeppe
Copy link
Contributor

mkoeppe commented Oct 29, 2021

sage.functions is the home for symbolic functions; this package will not be included in the distribution package sagemath-standard-no-symbolics (#32601), nor in smaller distributions such as sagemath-polyhedra (#32432).

sage.functions.log.log is not a symbolic function but only wrapper def that either delegates to a symbolic function or calls a method. We move it to sage.misc.functional so that non-symbolic uses of it do not pull in symbolics; and change these imports.

There is already a function log in sage.misc.functional, deprecated since #19444 (Sage 8.1). We replace it.

We keep a non-deprecated import of log in sage.functions.log for symbolic uses.

Similar to what was done in #32717 for sqrt.

CC: @tscrim @jhpalmieri @fchapoton @seblabbe @videlec @dimpase

Component: refactoring

Author: Matthias Koeppe

Branch/Commit: 238e97e

Reviewer: Dima Pasechnik

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

@mkoeppe mkoeppe added this to the sage-9.5 milestone Oct 29, 2021
@mkoeppe
Copy link
Contributor Author

mkoeppe commented Oct 29, 2021

Author: Matthias Koeppe

@mkoeppe

This comment has been minimized.

@mkoeppe mkoeppe changed the title sage.functions.log.log: Move to sage.misc.functions sage.functions.log.log: Move to sage.misc.functional Oct 29, 2021
@mkoeppe
Copy link
Contributor Author

mkoeppe commented Oct 29, 2021

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Oct 29, 2021

Commit: 0734119

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Oct 29, 2021

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

5798a1csrc/sage/coding/code_bounds.py: Import log from sage.misc.functional, add # optional - sage.symbolic to some doctests
0734119src/sage/graphs/generators/distance_regular.pyx: Import log from sage.misc.functional

@mkoeppe

This comment has been minimized.

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Oct 29, 2021

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

796b48bsrc/sage/coding/kasami_codes.pyx: Import log from sage.misc.functional
0f775a2src/sage/misc/dev_tools.py: Update doctest regarding import_statements('log')

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Oct 29, 2021

Changed commit from 0734119 to 0f775a2

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Oct 29, 2021

Changed commit from 0f775a2 to 238e97e

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Oct 29, 2021

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

6be920asrc/sage/combinat/words/finite_word.py: Change import of log for symbolic use to import from sage.functions.log
238e97esrc/sage/matrix/operation_table.py: Change non-symbolic use of log to import from sage.misc.functional

@mkoeppe
Copy link
Contributor Author

mkoeppe commented Oct 30, 2021

comment:9

Green bot, please review

@dimpase
Copy link
Member

dimpase commented Oct 30, 2021

Reviewer: Dima Pasechnik

@dimpase
Copy link
Member

dimpase commented Oct 30, 2021

comment:10

lgtm

@jhpalmieri
Copy link
Member

comment:11

It would be a little cleaner if the import in combinat/words/finite_word.py were from sage.misc.functional import log.

@mkoeppe
Copy link
Contributor Author

mkoeppe commented Oct 31, 2021

comment:12

I kept this one (in topological_entropy) as an import from sage.functions.log because a symbolic result is expected, as the doctests illustrate

@mkoeppe
Copy link
Contributor Author

mkoeppe commented Oct 31, 2021

comment:13

Thanks for the review!

@vbraun
Copy link
Member

vbraun commented Nov 2, 2021

@vbraun vbraun closed this as completed in ef03d44 Nov 2, 2021
mkoeppe added a commit to mkoeppe/sage that referenced this issue Feb 17, 2023
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