Opened 9 months ago

Closed 9 months ago

#32652 closed enhancement (fixed)

sage.geometry.polyhedron: Mark doctests # optional - sage.rings.number_field

Reported by: mkoeppe Owned by:
Priority: major Milestone: sage-9.5
Component: refactoring Keywords:
Cc: gh-kliem, tscrim Merged in:
Authors: Matthias Koeppe Reviewers: Jonathan Kliem
Report Upstream: N/A Work issues:
Branch: 1400654 (Commits, GitHub, GitLab) Commit: 14006541f2adbeb76bdc032d8152c9badf3e6226
Dependencies: #32614 Stopgaps:

Status badges

Description (last modified by mkoeppe)

(cherry-picked from #32432)

Change History (14)

comment:1 Changed 9 months ago by mkoeppe

  • Authors set to Matthias Koeppe
  • Dependencies set to #32614

comment:2 Changed 9 months ago by mkoeppe

  • Branch set to u/mkoeppe/sage_geometry_polyhedron__mark_doctests___optional___sage_rings_number_field

comment:3 Changed 9 months ago by mkoeppe

  • Cc gh-kliem tscrim added
  • Commit set to b84efa55878dfc84a75887a7c4669a489491d838
  • Description modified (diff)
  • Status changed from new to needs_review

Last 10 new commits:

​654d09csage.features.sagemath: Change sage_optional_tags to sage_features
​f63a7d0src/sage/features/: Move features depending on optional packages to separate files
​be9e715Merge #32614
​c0e73d8src/sage/geometry/polyhedron/double_description.py: Add # optional - sage.rings.number_field
​49658e0src/sage/geometry/polyhedron/backend_normaliz.py: Mark non-rational examples # optional - sage.rings.number_field
​a47c25bsrc/sage/geometry/polyhedron/constructor.py: Mark doctests # optional - sage.rings.number_field
​9ab8040src/sage/geometry/polyhedron/backend_field.py: Mark doctests # optional - sage.rings.number_field
​ce90678src/sage/geometry/polyhedron/base_ZZ.py: Mark a doctest # optional - sage.rings.number_field
​607598asrc/sage/geometry/polyhedron/base.py: Mark more doctests # optional - sage.rings.number_field
​b84efa5src/sage/geometry/polyhedron/base.py: Mark more doctests # optional - sage.rings.number_field

comment:4 Changed 9 months ago by git

  • Commit changed from b84efa55878dfc84a75887a7c4669a489491d838 to 1dee580c54c956d81f17f0fadd80e6e6ed9947f0

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

​1dee580src/sage/geometry/polyhedron/{base,backend_field}.py: Add some missing # optional

comment:5 follow-up: Changed 9 months ago by gh-kliem

Does sage.symbolic imply sage.rings.number_field? 

-    sage: Polyhedron([(0,0), (1,0), (1/2, sqrt(3)/2)])
+    sage: Polyhedron([(0,0), (1,0), (1/2, sqrt(3)/2)])                          # optional - sage.symbolic
     Traceback (most recent call last):
     ...
     ValueError: no default backend for computations with Symbolic Ring
 
-    sage: SR.is_exact()
+    sage: SR.is_exact()                                                         # optional - sage.symbolic
     False

comment:6 Changed 9 months ago by gh-kliem

  • Reviewers set to Jonathan Kliem

comment:7 in reply to: ↑ 5 Changed 9 months ago by mkoeppe

Replying to gh-kliem:

Does sage.symbolic imply sage.rings.number_field?

No, but number fields are not used in this example

comment:8 Changed 9 months ago by mkoeppe

Although here:

  • src/sage/geometry/polyhedron/constructor.py

    a b equilateral triangle whose vertex coordinates involve `\sqrt{3}`. An 
    177177exact way to work with roots in Sage is the :mod:`Algebraic Real Field
    178178<sage.rings.qqbar>` ::
    179179
    180     sage: triangle = Polyhedron([(0,0), (1,0), (1/2, sqrt(3)/2)], base_ring=AA)
    181     sage: triangle.Hrepresentation()
     180    sage: triangle = Polyhedron([(0,0), (1,0), (1/2, sqrt(3)/2)], base_ring=AA) # optional - sage.rings.number_field
     181    sage: triangle.Hrepresentation()                                            # optional - sage.rings.number_field
    182182    (An inequality (-1, -0.5773502691896258?) x + 1 >= 0,
    183183     An inequality (1, -0.5773502691896258?) x + 0 >= 0,
    184184     An inequality (0, 1.154700538379252?) x + 0 >= 0)

I should have marked it optional - sage.rings.number_field sage.symbolic

comment:9 Changed 9 months ago by git

  • Commit changed from 1dee580c54c956d81f17f0fadd80e6e6ed9947f0 to 1bb9d96fce8964851245aee75d25a1ab793878cb

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

​1bb9d96src/sage/geometry/polyhedron/constructor.py: More # optional - sage.symbolic

comment:10 Changed 9 months ago by gh-kliem

  • Status changed from needs_review to positive_review

LGTM.

comment:11 Changed 9 months ago by mkoeppe

Thank you!

comment:12 Changed 9 months ago by git

  • Commit changed from 1bb9d96fce8964851245aee75d25a1ab793878cb to 14006541f2adbeb76bdc032d8152c9badf3e6226
  • Status changed from positive_review to needs_review

Branch pushed to git repo; I updated commit sha1 and set ticket back to needs_review. New commits:

​4558791Merge tag '9.5.beta3' into t/32614/features_and_optional_tags_for_sage_subset_distributions
​1400654Merge #32614

comment:13 Changed 9 months ago by mkoeppe

  • Status changed from needs_review to positive_review

comment:14 Changed 9 months ago by vbraun

  • Branch changed from u/mkoeppe/sage_geometry_polyhedron__mark_doctests___optional___sage_rings_number_field to 14006541f2adbeb76bdc032d8152c9badf3e6226
  • Resolution set to fixed
  • Status changed from positive_review to closed
Note: See TracTickets for help on using tickets.