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

Remove unnecessary uses of SR and symbolic functions in sage.geometry (except .hyperbolic_space) #32416

Closed
mkoeppe opened this issue Aug 24, 2021 · 18 comments

Comments

@mkoeppe
Copy link
Contributor

mkoeppe commented Aug 24, 2021

(and except sage.geometry.riemannian_manifolds, which is removed in #32228)

Similar to what is done in #32411.

This is part of Meta-ticket #29705 (modularization) - to remove unnecessary dependencies on the distribution sagemath-symbolics (#31695)

CC: @kliem @fchapoton

Component: refactoring

Author: Jonathan Kliem

Branch/Commit: c40df1a

Reviewer: Matthias Koeppe

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

@mkoeppe mkoeppe added this to the sage-9.5 milestone Aug 24, 2021
@kliem
Copy link
Contributor

kliem commented Aug 27, 2021

comment:1

I'm working on this at the moment. Using sympy.symbols is ok, right?
Probably yes, because geometry depends on categories.sets_cat, which depends on sympy.

I'm just too lazy to reinvent the wheel and was looking for something that prints linear combinations of symbolic variables as well. Of course it is not too hard to write this yourself...

@mkoeppe
Copy link
Contributor Author

mkoeppe commented Aug 27, 2021

comment:2

Hm... what do you need this for? Are you just looking for repr_lincomb?

@kliem
Copy link
Contributor

kliem commented Aug 27, 2021

comment:3

Yes, that is exactly what I was looking for.

@kliem
Copy link
Contributor

kliem commented Aug 27, 2021

@kliem
Copy link
Contributor

kliem commented Aug 27, 2021

comment:4

This is a partial solution.

  • Some doctests need to be marked optional yet. In this ticket?
  • sage.geometry.polyhedron.base.Polyhedron_base.affine_hull_manifold is missing yet.
  • sage.geometry.hyperplane_arrangement.plot is missing. Not sure that this even works.

New commits:

d3479c7remove some of symbolics from sage.geometry

@kliem
Copy link
Contributor

kliem commented Aug 27, 2021

Author: Jonathan Kliem, ...

@kliem
Copy link
Contributor

kliem commented Aug 27, 2021

Commit: d3479c7

@mkoeppe
Copy link
Contributor Author

mkoeppe commented Aug 27, 2021

comment:5

These changes look good to me so far.

@mkoeppe
Copy link
Contributor Author

mkoeppe commented Aug 27, 2021

comment:6

Replying to @kliem:

  • Some doctests need to be marked optional yet. In this ticket?

We don't really have "optional" tags for things like sagemath_symbolics yet. You could add it to the beginning of sage.doctest.control (auto_optional_tags) but a more general solution is needed in the long term, see #30746

@mkoeppe
Copy link
Contributor Author

mkoeppe commented Sep 30, 2021

Changed author from Jonathan Kliem, ... to Jonathan Kliem

@mkoeppe
Copy link
Contributor Author

mkoeppe commented Sep 30, 2021

comment:8

Let's see what the patchbot thinks

@mkoeppe
Copy link
Contributor Author

mkoeppe commented Sep 30, 2021

comment:9

1 doctest failure from the changed latex whitespace in representation printing

sage -t --long --random-seed=0 src/doc/en/thematic_tutorials/geometry/tips.rst
**********************************************************************
File "src/doc/en/thematic_tutorials/geometry/tips.rst", line 82, in doc.en.thematic_tutorials.geometry.tips
Failed example:
    print(TCube.Hrepresentation_str(latex=True))

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Sep 30, 2021

Changed commit from d3479c7 to c40df1a

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Sep 30, 2021

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

ef19b5aMerge branch 'u/gh-kliem/remove_symbolics_from_geometry' of git://trac.sagemath.org/sage into u/gh-kliem/remove_symbolics_from_geometry
c40df1afix doctest

@mkoeppe
Copy link
Contributor Author

mkoeppe commented Oct 3, 2021

Reviewer: Matthias Koeppe

@mkoeppe
Copy link
Contributor Author

mkoeppe commented Oct 3, 2021

comment:11

LGTM, thanks.

@mkoeppe
Copy link
Contributor Author

mkoeppe commented Oct 3, 2021

comment:12

Replying to @mkoeppe:

We don't really have "optional" tags for things like sagemath_symbolics yet.

Now provided by #32614

@vbraun
Copy link
Member

vbraun commented Oct 10, 2021

Changed branch from u/gh-kliem/remove_symbolics_from_geometry to c40df1a

@vbraun vbraun closed this as completed in d349229 Oct 10, 2021
mkoeppe added a commit to mkoeppe/sage that referenced this issue Feb 12, 2023
mkoeppe added a commit to mkoeppe/sage that referenced this issue Feb 12, 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

3 participants