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

Sympy minus infinity doesn't convert to Sage #12345

Closed
kcrisman opened this issue Jan 23, 2012 · 12 comments
Closed

Sympy minus infinity doesn't convert to Sage #12345

kcrisman opened this issue Jan 23, 2012 · 12 comments

Comments

@kcrisman
Copy link
Member

sage: a,b = oo, -oo
sage: a._sympy_()
oo
sage: b._sympy_()
--------------------------------------------
AttributeError                            Traceback (most recent call last)

/Users/.../sage-4.7.2/local/lib/python2.6/site-packages/sage/structure/element.so in sage.structure.element.Element.__getattr__ (sage/structure/element.c:2840)()

AttributeError: 'MinusInfinity' object has no attribute '_sympy_'

See this ask.sagemath.org question for background.

CC: @certik @rwst

Component: symbolics

Keywords: sd40.5

Author: Douglas McNeil

Branch/Commit: u/akhi/sympy_minus_infinity_doesn_t_convert_to_sage @ 52db42b

Reviewer: Karl-Dieter Crisman

Merged: sage-5.1.beta4

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

@sagetrac-dsm
Copy link
Mannequin

sagetrac-dsm mannequin commented May 26, 2012

Changed keywords from none to sd40.5

@sagetrac-dsm
Copy link
Mannequin

sagetrac-dsm mannequin commented May 26, 2012

comment:1

Straightforward fix and test of the original case as an integral.

@sagetrac-dsm sagetrac-dsm mannequin added the s: needs review label May 26, 2012
@kcrisman
Copy link
Member Author

comment:2

Umm, even without this patch I get

sage: import sympy
sage: bool(SR(-oo) == -sympy.oo)
True
sage: bool(-oo == -sympy.oo)
True

Am I missing something? How is that an indirect doctest?

That said, the integral works fine now and the tests pass.

@kcrisman
Copy link
Member Author

Reviewer: Karl-Dieter Crisman

@kcrisman
Copy link
Member Author

Author: Douglas McNeil

@sagetrac-dsm
Copy link
Mannequin

sagetrac-dsm mannequin commented May 26, 2012

comment:3

Ah, those were merely mirroring the +oo cases. Really they were only testing whether the negation worked; it was the integral test which was really verifying the success. However, we can test it explicitly by adding.

    sage: bool((-oo)._sympy_() == -sympy.oo)
    True

which fails without the patch. Modified to incorporate this.

@kcrisman
Copy link
Member Author

comment:4

Positive review.

@jdemeyer
Copy link

Attachment: trac_12345_minusinfinity_sympyfication.patch.gz

add _sympy_ method to MinusInfinity

@jdemeyer
Copy link

Merged: sage-5.1.beta4

@sagetrac-akhi
Copy link
Mannequin

sagetrac-akhi mannequin commented Mar 18, 2015

@kcrisman
Copy link
Member Author

comment:7

I think this ticket is already fixed?

@kcrisman
Copy link
Member Author

Commit: 52db42b

vbraun pushed a commit that referenced this issue Mar 26, 2023
    
<!-- Please provide a concise, informative and self-explanatory title.
-->
<!-- Don't put issue numbers in the title. Put it in the Description
below. -->

### 📚 Description

Made small edits to the PR template, making it concise and tidy overall.

<!-- 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 #12345". -->
<!-- If your change requires a documentation PR, please link it
appropriately. -->

### 📝 Checklist

<!-- Put an `x` in all the boxes that apply. It should be `[x]` not `[x
]`. -->

- [x] The title is concise, informative, and self-explanatory.
- [x] The description explains in detail what this PR is about.
- [ ] 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
Depends on #12345: short description why this is a dependency
Depends on #34567: ...
-->

<!-- If you're unsure about any of these, don't hesitate to ask. We're
here to help! -->
    
URL: #35141
Reported by: Kwankyu Lee
Reviewer(s): Kwankyu Lee, Tobias Diez
vbraun pushed a commit that referenced this issue Apr 6, 2023
    
<!-- Please provide a concise, informative and self-explanatory title.
-->
<!-- Don't put issue numbers in the title. Put it in the Description
below. -->
<!-- For example, instead of "Fixes #12345", use "Add a new method to
multiply two integers" -->

### 📚 Description

Partial fix for pycodestyle E221, done using autopep8.

E221 multiple spaces before operator

<!-- 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 #12345". -->
<!-- If your change requires a documentation PR, please link it
appropriately. -->

### 📝 Checklist

<!-- Put an `x` in all the boxes that apply. It should be `[x]` not `[x
]`. -->

- [x] The title is concise, informative, and self-explanatory.
- [x] The description explains in detail what this PR is about.
- [ ] 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
- #12345: short description why this is a dependency
- #34567: ...
-->

<!-- If you're unsure about any of these, don't hesitate to ask. We're
here to help! -->
    
URL: #35355
Reported by: Frédéric Chapoton
Reviewer(s): Matthias Köppe
vbraun pushed a commit that referenced this issue Apr 6, 2023
    
<!-- Please provide a concise, informative and self-explanatory title.
-->
<!-- Don't put issue numbers in the title. Put it in the Description
below. -->
<!-- For example, instead of "Fixes #12345", use "Add a new method to
multiply two integers" -->

### 📚 Description

The update #35184 broke the docs
upload. Hopefully this is fixed with this PR.

<!-- 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 #12345". -->
<!-- If your change requires a documentation PR, please link it
appropriately. -->

### 📝 Checklist

<!-- Put an `x` in all the boxes that apply. It should be `[x]` not `[x
]`. -->

- [x] The title is concise, informative, and self-explanatory.
- [x] The description explains in detail what this PR is about.
- [ ] 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
- #12345: short description why this is a dependency
- #34567: ...
-->

<!-- If you're unsure about any of these, don't hesitate to ask. We're
here to help! -->
    
URL: #35356
Reported by: Tobias Diez
Reviewer(s): Dima Pasechnik, Kwankyu Lee
vbraun pushed a commit that referenced this issue Apr 6, 2023
    
<!-- Please provide a concise, informative and self-explanatory title.
-->
<!-- Don't put issue numbers in the title. Put it in the Description
below. -->
<!-- For example, instead of "Fixes #12345", use "Add a new method to
multiply two integers" -->

### 📚 Description

<!-- Describe your changes here in detail. -->
Follow-up from #35322.
<!-- 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 #12345". -->
<!-- If your change requires a documentation PR, please link it
appropriately. -->

### 📝 Checklist

<!-- Put an `x` in all the boxes that apply. It should be `[x]` not `[x
]`. -->

- [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
- #12345: short description why this is a dependency
- #34567: ...
-->

<!-- If you're unsure about any of these, don't hesitate to ask. We're
here to help! -->
    
URL: #35366
Reported by: Matthias Köppe
Reviewer(s): Gonzalo Tornaría
vbraun pushed a commit that referenced this issue Apr 6, 2023
    
<!-- Please provide a concise, informative and self-explanatory title.
-->
<!-- Don't put issue numbers in the title. Put it in the Description
below. -->
<!-- For example, instead of "Fixes #12345", use "Add a new method to
multiply two integers" -->

### 📚 Description

This is a follow-up on:
- #35110

As preparation for #35322, which is changing more packages to implicit
namespace packages, we remove `.all` imports from these packages
throughout the Sage library.

This is part of:
- #29705

<!-- 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 #12345". -->
<!-- If your change requires a documentation PR, please link it
appropriately. -->

### 📝 Checklist

<!-- Put an `x` in all the boxes that apply. It should be `[x]` not `[x
]`. -->

- [x] The title is concise, informative, and self-explanatory.
- [x] 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
- #12345: short description why this is a dependency
- #34567: ...
-->
- Depends on #35418
- Depends on #35358
<!-- If you're unsure about any of these, don't hesitate to ask. We're
here to help! -->
    
URL: #35372
Reported by: Matthias Köppe
Reviewer(s): Gonzalo Tornaría
vbraun pushed a commit that referenced this issue Apr 6, 2023
    
<!-- Please provide a concise, informative and self-explanatory title.
-->
<!-- Don't put issue numbers in the title. Put it in the Description
below. -->
<!-- For example, instead of "Fixes #12345", use "Add a new method to
multiply two integers" -->

### 📚 Description
openblas 0.3.22 is broken, see:
- #35371
- scipy/scipy#18208
- OpenMathLib/OpenBLAS#3976

We reject it.

<!-- 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 #12345". -->
<!-- If your change requires a documentation PR, please link it
appropriately. -->

### 📝 Checklist

<!-- Put an `x` in all the boxes that apply. It should be `[x]` not `[x
]`. -->

- [x] The title is concise, informative, and self-explanatory.
- [x] 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
- #12345: short description why this is a dependency
- #34567: ...
-->

<!-- If you're unsure about any of these, don't hesitate to ask. We're
here to help! -->
    
URL: #35377
Reported by: Matthias Köppe
Reviewer(s): John H. Palmieri
vbraun pushed a commit to vbraun/sage that referenced this issue Apr 25, 2024
    
The DFT for the SGA throws a ZeroDivisionError when p|n. Uses the
idempotents as computed in [Murphy '83] to build the Pierce
decomposition of the symmetric group algebra over a finite field. There
is a homomorphic projection onto each block via v |--> v*e_i. The change
of basis matrix is the modular Fourier transform, and works even when
p|n.

<!-- ^ Please provide a concise and informative title. -->
<!-- ^ Don't put issue numbers in the title, do this in the PR
description below. -->
<!-- ^ For example, instead of "Fixes sagemath#12345" use "Introduce new method
to calculate 1 + 2". -->
<!-- v Describe your changes below in detail. -->
<!-- v Why is this change required? What problem does it solve? -->
<!-- v If this PR resolves an open issue, please link to it here. For
example, "Fixes sagemath#12345". -->



### 📝 Checklist

<!-- Put an `x` in all the boxes that apply. -->

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

### ⌛ Dependencies

<!-- List all open PRs that this PR logically depends on. For example,
-->
<!-- - sagemath#12345: short description why this is a dependency -->
<!-- - sagemath#34567: ... -->
    
URL: sagemath#37757
Reported by: Jackson Walters
Reviewer(s): grhkm21, Jackson Walters, Sebastian A. Spindler, Travis Scrimshaw
vbraun pushed a commit to vbraun/sage that referenced this issue Apr 25, 2024
…inite fields

    
<!-- ^ Please provide a concise and informative title. -->
<!-- ^ Don't put issue numbers in the title, do this in the PR
description below. -->
<!-- ^ For example, instead of "Fixes sagemath#12345" use "Introduce new method
to calculate 1 + 2". -->
<!-- v Describe your changes below in detail. -->
<!-- v Why is this change required? What problem does it solve? -->
<!-- v If this PR resolves an open issue, please link to it here. For
example, "Fixes sagemath#12345". -->

Many classical Lie algebras are unable to be constructed because they
multiply a rational number times an element in the base ring. If the
base ring is, say, `GF(3)`, then there is no coercion (since not every
element is defined), so we need to force all elements to be in the base
ring during the computation (which might perform a division that goes to
the fraction field, but it will eventually brings us back to a
$\mathbb{Z}$-basis).

Before, this would fail with a `QQ` to `GF(3)` coercion error:
```
sage: sl4 = lie_algebras.sl(GF(3), 4)
```

### 📝 Checklist

<!-- Put an `x` in all the boxes that apply. -->

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

### ⌛ Dependencies

<!-- List all open PRs that this PR logically depends on. For example,
-->
<!-- - sagemath#12345: short description why this is a dependency -->
<!-- - sagemath#34567: ... -->
    
URL: sagemath#37773
Reported by: Travis Scrimshaw
Reviewer(s): Frédéric Chapoton, Travis Scrimshaw
vbraun pushed a commit to vbraun/sage that referenced this issue Apr 25, 2024
…ss the TestSuite

<!-- ^ Please provide a concise and informative title. -->
<!-- ^ Don't put issue numbers in the title, do this in the PR
description below. -->
<!-- ^ For example, instead of "Fixes sagemath#12345" use "Introduce new method
to calculate 1 + 2". -->
<!-- v Describe your changes below in detail. -->
<!-- v Why is this change required? What problem does it solve? -->
<!-- v If this PR resolves an open issue, please link to it here. For
example, "Fixes sagemath#12345". -->

The output of `xgcd` does not match the `gcd` output, but this is
required by sagemath#17671. We change the computation to make these match. We
also fix another bug as it should also take non-Laurent polynomial
inputs to `xgcd`.

We also implement the `euclidean_domain()` method.

### 📝 Checklist

<!-- Put an `x` in all the boxes that apply. -->

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

### ⌛ Dependencies

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

URL: sagemath#37778
Reported by: Travis Scrimshaw
Reviewer(s): Enrique Manuel Artal Bartolo, Travis Scrimshaw
vbraun pushed a commit to vbraun/sage that referenced this issue Apr 26, 2024
…oring out in degenerate cases

    
<!-- ^ Please provide a concise and informative title. -->
<!-- ^ Don't put issue numbers in the title, do this in the PR
description below. -->
<!-- ^ For example, instead of "Fixes sagemath#12345" use "Introduce new method
to calculate 1 + 2". -->
<!-- v Describe your changes below in detail. -->
<!-- v Why is this change required? What problem does it solve? -->
<!-- v If this PR resolves an open issue, please link to it here. For
example, "Fixes sagemath#12345". -->

Replaced the error raised by `SetPartition` in a degenerate case (see
below) by an empty iterator, to match the behaviour of similar functions
such as `Partitions` and `OrderedSetPartitions`. Old behaviour:
```Python
sage: list( Partitions(3,length=4) )
[]
sage: list( OrderedSetPartitions(range(3),4) )
[]
sage: list( SetPartitions(range(3),4) )
...
ValueError: part must be <= len(set)
```
New behaviour
```Python
sage: list( SetPartitions(range(3),4) )
[]
```
This solves issue sagemath#37643.

### 📝 Checklist

<!-- Put an `x` in all the boxes that apply. -->

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

### ⌛ Dependencies

<!-- List all open PRs that this PR logically depends on. For example,
-->
<!-- - sagemath#12345: short description why this is a dependency -->
<!-- - sagemath#34567: ... -->
    
URL: sagemath#37781
Reported by: Nolord
Reviewer(s): Travis Scrimshaw
vbraun pushed a commit to vbraun/sage that referenced this issue Apr 26, 2024
    
<!-- ^ Please provide a concise and informative title. -->
<!-- ^ Don't put issue numbers in the title, do this in the PR
description below. -->
<!-- ^ For example, instead of "Fixes sagemath#12345" use "Introduce new method
to calculate 1 + 2". -->
<!-- v Describe your changes below in detail. -->
<!-- v Why is this change required? What problem does it solve? -->
<!-- v If this PR resolves an open issue, please link to it here. For
example, "Fixes sagemath#12345". -->

Since we are going to work with a matrix $f^{\lambda} \times
f^{\lambda}$ (recall $f^{\lambda}$ is the number of standard Young
tableaux of shape $\lambda$), then we should just store the list of
standard tableaux rather than iterating over them repeatedly. This
results in a major speedup:
```
sage: from sage.combinat.specht_module import simple_module_rank
sage: %time simple_module_rank([5,3,2], GF(2))
CPU times: user 1.34 s, sys: 35.7 ms, total: 1.38 s
Wall time: 1.38 s
200
```
versus before
```
sage: %time simple_module_rank([5,3,2], GF(2))
CPU times: user 4.97 s, sys: 65 ms, total: 5.03 s
Wall time: 5.06 s
200
```

### 📝 Checklist

<!-- Put an `x` in all the boxes that apply. -->

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

### ⌛ Dependencies

<!-- List all open PRs that this PR logically depends on. For example,
-->
<!-- - sagemath#12345: short description why this is a dependency -->
<!-- - sagemath#34567: ... -->
    
URL: sagemath#37784
Reported by: Travis Scrimshaw
Reviewer(s): Darij Grinberg
vbraun pushed a commit to vbraun/sage that referenced this issue Apr 26, 2024
… messages

    
<!-- ^ Please provide a concise and informative title. -->
<!-- ^ Don't put issue numbers in the title, do this in the PR
description below. -->
<!-- ^ For example, instead of "Fixes sagemath#12345" use "Introduce new method
to calculate 1 + 2". -->
<!-- v Describe your changes below in detail. -->
<!-- v Why is this change required? What problem does it solve? -->
<!-- v If this PR resolves an open issue, please link to it here. For
example, "Fixes sagemath#12345". -->

A minor follow-up after sagemath#37391:

Seen for example in
https://github.com/sagemath/sage/actions/runs/8626379356/job/23644513913
?pr=37570#step:11:3227:
```
sagemath#24 4091.8 /sage/build/bin/sage-logger: line 66: /usr/bin/time: No such
file or directory
sagemath#24 4091.8 [sagemath_doc_html-none] installing. Log file:
/sage/logs/pkgs/sagemath_doc_html-none.log
sagemath#24 5807.5 cat: /sage/logs/pkgs/sagemath_doc_html-none.time: No such
file or directory
```
We suppress these (harmless) messages which show up when `/usr/bin/time`
is not available.

### 📝 Checklist

<!-- Put an `x` in all the boxes that apply. -->

- [x] The title is concise and informative.
- [x] The description explains in detail what this PR is about.
- [ ] 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. For example,
-->
<!-- - sagemath#12345: short description why this is a dependency -->
<!-- - sagemath#34567: ... -->
    
URL: sagemath#37785
Reported by: Matthias Köppe
Reviewer(s): David Ayotte, Gonzalo Tornaría, Matthias Köppe
vbraun pushed a commit to vbraun/sage that referenced this issue Apr 28, 2024
…plane curves

    
The goal of this PR is to add some features to the computation of
fundamental groups of complements of the plane curves introduced in
``Sagemath``  by @miguelmarco and his optional package ``sirocco``. It
also corrects a bug introduced in
sagemath#35376 and cleans some code.

We add the following information: given a tuple of curves we retrieve
the information of meridians of each curve as words in the fundamental
group, and also of the strands of the braid monodromy. We also add the
possibility of excluding the vertical lines from the computation of
braid monodromy (as far as there is no vertical asymptote) which makes
its computation faster.

To achieve this goal we introduce new classes
`AffinePlaneCurveArrangements` and `ProjectivePlaneCurveArrangements`.
For now, they serve only to compute the fundamental groups with the
meridian information (and braid monodromy and strand information for the
affine curves) but more methods may be constructed in the future. The
structure of these classes follows the one of `HyperplaneArrangements`,
with one difference, the order is important. This is why we also
introduce the class of `OrderedHyperplaneArrangements`, where the
fundamental group is computed. We have also introduce a new method for
hyperplane arrangements, `jhyperplane_section`, in order to compute the
fundamental group also for hyperplane arrangements and not only for line
arrangements.

These are the main changes:

- schemes/curves/affine_curve.py
     - Introducion of  some auxiliary new methods:
`has_vertical_asymptote`, `is_vertical_line`
     - Small update of `fundamental_group`
- schemes/curves/projective_curve.py. Some trivial cases were not
considered in `fundamental_group`.
- schemes/curves/plane_curve_arrangement.py. New file with the new
classes.
- schemes/curves/zariski_vankampen.py
    - For consistency some lists are converted into tuples.
    - Some functions take a new keyword to take into account the option
of eliminating the vertical lines for the computation of the braid
monodromy.
    - In `braid_in_segment`, correction of the bug introduced in the
previous PR (the parameters must be in the number field embedded in
`QQbar`).
    - The function `geometric_basis` has been changed for designed and
to cover some cases where the former one failed.
    - A new function `vertical_lines_in_braidmon` to isolate vertical
lines if needed.
    - The function `braid_monodromy` has been cleaned and now it takes
into account the strand information.
    -
- geometry/hyperplane_arrangement/arrangement.py
     - Introducion of the class of `OrderedHyperplaneArrangements`. The
hyperplanes are not sorted in this class and the introduction order is
respected. It forces to update some methods.
     - Five new methods for this new class: `hyperplane_section`,
`affine_fundamental_group`, `affine_meridians`,
`projective_fundamental_group`, and `projective_meridians`.



<!-- 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.
- [X] The description explains in detail what this PR is about.
- [X] I have created tests covering the changes.
- [X] I have updated the documentation accordingly.
    
URL: sagemath#36768
Reported by: Enrique Manuel Artal Bartolo
Reviewer(s): Enrique Manuel Artal Bartolo, Frédéric Chapoton, Travis Scrimshaw
vbraun pushed a commit to vbraun/sage that referenced this issue Apr 28, 2024
…gemath-environment, sage-setup, sage-sws2rst for PyPI

    
<!-- ^^^^^
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? -->
We already build platform wheels for
**sagemath-{objects,categories,repl}** for PyPI using cibuildwheel.
Here we add building of platform-independent wheels for **sagemath-
environment** and **sage-setup** and **sage-sws2rst** to the GH Actions
workflow. (They are needed, for example, for pyodide / jupyterlite.)

To test locally: `make pypi-noarch-wheels`

We also update actions/upload-artifact, actions/download-artifact to v4.
This requires a restructuring, as we can no longer upload the wheels for
different architectures (built by separate matrix jobs) to the same
artifact: https://github.com/actions/upload-artifact?tab=readme-ov-
file#not-uploading-to-the-same-artifact
Instead we upload them as separate artifacts and deploy them to PyPI
directly from the job that built them.

This is:
- part of sagemath#31251

<!-- 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.
- [x] The description explains in detail what this PR is about.
- [ ] 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#37099
Reported by: Matthias Köppe
Reviewer(s): Kwankyu Lee, Matthias Köppe
vbraun pushed a commit to vbraun/sage that referenced this issue Apr 28, 2024
…fancy_pypi_readme 24.1.0, platformdirs 4.2.0, packaging 24.0, trove_classifiers 2024.4.10, wheel 0.43.0

    
<!-- ^^^^^
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.
- [ ] 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#37277
Reported by: Matthias Köppe
Reviewer(s): Kwankyu Lee, Matthias Köppe
vbraun pushed a commit to vbraun/sage that referenced this issue Apr 28, 2024
…rt PURLs `pkg:pypi/DISTRO-NAME`, obtain dependencies of wheels from PyPI

    
<!-- ^ Please provide a concise and informative title. -->
<!-- ^ Don't put issue numbers in the title, do this in the PR
description below. -->
<!-- ^ For example, instead of "Fixes sagemath#12345" use "Introduce new method
to calculate 1 + 2". -->
<!-- v Describe your changes below in detail. -->
<!-- v Why is this change required? What problem does it solve? -->
<!-- v If this PR resolves an open issue, please link to it here. For
example, "Fixes sagemath#12345". -->

We make it possible to refer to Python packages via their PURL (see
[draft PEP 725](https://peps.python.org/pep-0725/#concrete-package-
specification-through-purl)) instead of their SPKG name.

For now a string of the form `pkg:pypi/DISTRO-NAME` is simply a nickname
for the (unique) SPKG that has DISTRO-NAME in their `install-
requires.txt` or `requirements.txt`. The scheme can also be omitted:
`pypi/DISTRO-NAME` also works. And we also map `pkg:generic/PACKAGE-
NAME` to `PACKAGE_NAME`.

Based on code by @culler, `sage --package create --pypi` now also fills
`dependencies` from the PyPI metadata of wheel packages. When some of
the Python dependencies obtained in this way do not have SPKGs yet, they
are also automatically created.

- Preparation for sagemath#31136.
- Split out from sagemath#37250.


### 📝 Checklist

<!-- Put an `x` in all the boxes that apply. -->

- [x] The title is concise and informative.
- [x] The description explains in detail what this PR is about.
- [ ] 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. For example,
-->
<!-- - sagemath#12345: short description why this is a dependency -->
<!-- - sagemath#34567: ... -->
    
URL: sagemath#37500
Reported by: Matthias Köppe
Reviewer(s): Kwankyu Lee, Marc Culler, Matthias Köppe
vbraun pushed a commit to vbraun/sage that referenced this issue Apr 28, 2024
…representation`: Support constructing `Hom(CombinatorialFreeModule)` elements

    
<!-- ^ Please provide a concise and informative title. -->
<!-- ^ Don't put issue numbers in the title, do this in the PR
description below. -->
<!-- ^ For example, instead of "Fixes sagemath#12345" use "Introduce new method
to calculate 1 + 2". -->
<!-- v Describe your changes below in detail. -->
<!-- v Why is this change required? What problem does it solve? -->
<!-- v If this PR resolves an open issue, please link to it here. For
example, "Fixes sagemath#12345". -->

We use morphisms of `CombinatorialFreeModule`s (each of which has a
distinguished finite or enumerated basis indexed by arbitrary objects)
as matrices whose rows and columns are indexed by arbitrary objects
(`row_keys`, `column_keys`).

Example:
```
        sage: M = matrix([[1,2,3], [4,5,6]],
        ....:            column_keys=['a','b','c'], row_keys=['u','v']);
M
        Generic morphism:
          From: Free module generated by {'a', 'b', 'c'} over Integer
Ring
          To:   Free module generated by {'u', 'v'} over Integer Ring
```

Example application done here on the PR: The incidence matrix of a graph
or digraph. Returning it as a morphism instead of a matrix has the
benefit of keeping the vertices and edges with the result. This new
behavior is activated by special values for the existing parameters
`vertices` and `edges`.
```
            sage: D12 = posets.DivisorLattice(12).hasse_diagram()
            sage: phi_VE = D12.incidence_matrix(vertices=True,
edges=True); phi_VE
            Generic morphism:
              From: Free module generated by
                      {(1, 2), (1, 3), (2, 4), (2, 6), (3, 6), (4, 12),
(6, 12)}
                    over Integer Ring
              To:   Free module generated by {1, 2, 3, 4, 6, 12} over
Integer Ring
            sage: print(phi_VE._unicode_art_matrix())
                         (1, 2)  (1, 3)  (2, 4)  (2, 6)  (3, 6) (4, 12)
(6, 12)
                      1⎛     -1      -1       0       0       0       0
0⎞
                      2⎜      1       0      -1      -1       0       0
0⎟
                      3⎜      0       1       0       0      -1       0
0⎟
                      4⎜      0       0       1       0       0      -1
0⎟
                      6⎜      0       0       0       1       1       0
-1⎟
                     12⎝      0       0       0       0       0       1
1⎠
```



### 📝 Checklist

<!-- Put an `x` in all the boxes that apply. -->

- [x] The title is concise and informative.
- [x] The description explains in detail what this PR is about.
- [ ] 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. For example,
-->
<!-- - sagemath#12345: short description why this is a dependency -->
<!-- - sagemath#34567: ... -->
- Depends on sagemath#37607
- Depends on sagemath#37514
- Depends on sagemath#37606
- Depends on sagemath#37646
    
URL: sagemath#37692
Reported by: Matthias Köppe
Reviewer(s): gmou3
vbraun pushed a commit to vbraun/sage that referenced this issue Apr 28, 2024
…for finite dimensional Lie algebras

    
<!-- ^ Please provide a concise and informative title. -->
<!-- ^ Don't put issue numbers in the title, do this in the PR
description below. -->
<!-- ^ For example, instead of "Fixes sagemath#12345" use "Introduce new method
to calculate 1 + 2". -->
<!-- v Describe your changes below in detail. -->
<!-- v Why is this change required? What problem does it solve? -->
<!-- v If this PR resolves an open issue, please link to it here. For
example, "Fixes sagemath#12345". -->

We implement the upper central series and hypercenter of a finite
dimensional Lie algebra.

As part of the testing, we found improvements could be made to the
generic example (needed so the example would work with these tests): We
moved the `reduce()` method up to the category. Implemented methods
expected for the subalgebras. Better input verification. Elements are
now hashable. We had to work around the fact that `lift()` is serving
multiple roles as to the UEA and to the ambient space for a Lie
subalgebra. This functionality needs to be divided (I take full
responsibility for this bad design) by renaming `lift` to something for
the UEA map, but this is a significant change that should be done on a
followup PR.

We also fixed some bugs and improved efficiency with changing vectors
into Lie algebra elements.

### 📝 Checklist

<!-- Put an `x` in all the boxes that apply. -->

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

### ⌛ Dependencies

<!-- List all open PRs that this PR logically depends on. For example,
-->
<!-- - sagemath#12345: short description why this is a dependency -->
<!-- - sagemath#34567: ... -->
    
URL: sagemath#37693
Reported by: Travis Scrimshaw
Reviewer(s): Matthias Köppe, Travis Scrimshaw
vbraun pushed a commit to vbraun/sage that referenced this issue Apr 28, 2024
    
<!-- ^ Please provide a concise and informative title. -->
<!-- ^ Don't put issue numbers in the title, do this in the PR
description below. -->
<!-- ^ For example, instead of "Fixes sagemath#12345" use "Introduce new method
to calculate 1 + 2". -->
<!-- v Describe your changes below in detail. -->
<!-- v Why is this change required? What problem does it solve? -->
<!-- v If this PR resolves an open issue, please link to it here. For
example, "Fixes sagemath#12345". -->

Expanding the feature
https://github.com/sagemath/sage/wiki/Sage-10.3-Release-Tour#doctest-
failures-are-shown-in-the-files-changed-tab-on-prs
to cover doctests that crash the doctest runner.

An example (https://github.com/sagemath/sage/pull/37709/files)
<img width="1078" alt="Screenshot 2024-04-03 at 11 14 05 AM"
src="https://github.com/sagemath/sage/assets/8345221/de990f1f-069c-4fcf-
ac7e-ed915687533b">


### 📝 Checklist

<!-- Put an `x` in all the boxes that apply. -->

- [x] The title is concise and informative.
- [x] The description explains in detail what this PR is about.
- [ ] 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. For example,
-->
<!-- - sagemath#12345: short description why this is a dependency -->
<!-- - sagemath#34567: ... -->
    
URL: sagemath#37738
Reported by: Matthias Köppe
Reviewer(s): Kwankyu Lee
vbraun pushed a commit to vbraun/sage that referenced this issue Apr 28, 2024
… after sagemath#37022

    
<!-- ^ Please provide a concise and informative title. -->
<!-- ^ Don't put issue numbers in the title, do this in the PR
description below. -->
<!-- ^ For example, instead of "Fixes sagemath#12345" use "Introduce new method
to calculate 1 + 2". -->
<!-- v Describe your changes below in detail. -->
<!-- v Why is this change required? What problem does it solve? -->
<!-- v If this PR resolves an open issue, please link to it here. For
example, "Fixes sagemath#12345". -->

- Cherry-picked updates of testing infrastructure from sagemath#35095.
- Add upper version bounds to avoid a regression in 4.14.1
- Fixes sagemath#37734.
- Also removes some unnecessary runs of the "CI Linux Incremental"
workflow.

### 📝 Checklist

<!-- Put an `x` in all the boxes that apply. -->

- [x] The title is concise and informative.
- [ ] 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. For example,
-->
<!-- - sagemath#12345: short description why this is a dependency -->
<!-- - sagemath#34567: ... -->
    
URL: sagemath#37750
Reported by: Matthias Köppe
Reviewer(s): Kwankyu Lee
vbraun pushed a commit to vbraun/sage that referenced this issue Apr 28, 2024
…egarding deprecation

    
In PR sagemath#37721, I attempted to deprecate the ``ring`` keyword argument in
the ``matrix`` constructor
using ``@rename_keyword`` documented [here](https://doc.sagemath.org/htm
l/en/developer/coding_in_python.html#deprecation).
It did not work. The code compiled exactly as if the decorator wasn't
there.
I asked @mkoeppe why, he answered it "would be worth investigating and
documenting".
After some investigation, I came to the conclusion that developers
cannot use ``@rename_keyword`` and
other such decorators in Cython files.
My solution was to use ``deprecation_cython`` (this was accepted and
merged).
Since many other developers might end up having the exact same problem,
I simply suggest redirecting them
to ``deprecation_cython``, by adding what's necessary in the docs.
In this regard, I added a paragraph mentioning this in the Developer's
guide, as all of this was **totally absent** from documentation.

This way other developers won't need to investigate and spend time
debugging non-working code. :)

<!-- ^ Please provide a concise and informative title. -->
<!-- ^ Don't put issue numbers in the title, do this in the PR
description below. -->
<!-- ^ For example, instead of "Fixes sagemath#12345" use "Introduce new method
to calculate 1 + 2". -->
<!-- v Describe your changes below in detail. -->
<!-- v Why is this change required? What problem does it solve? -->
<!-- v If this PR resolves an open issue, please link to it here. For
example, "Fixes sagemath#12345". -->



### 📝 Checklist

<!-- Put an `x` in all the boxes that apply. -->

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

### ⌛ Dependencies

<!-- List all open PRs that this PR logically depends on. For example,
-->
<!-- - sagemath#12345: short description why this is a dependency -->
<!-- - sagemath#34567: ... -->
    
URL: sagemath#37807
Reported by: SandwichGouda
Reviewer(s): Matthias Köppe
vbraun pushed a commit to vbraun/sage that referenced this issue Apr 28, 2024
… in farey_symbol.pyx

    
See sagemath#37807 for discussion.
The keyword was renamed and is deprecated since 2013 : it is high-time
we get rid of the old version.
Besides, ``@rename_keyword`` does not work properly in Cython files (see
sagemath#37807 )

<!-- ^ Please provide a concise and informative title. -->
<!-- ^ Don't put issue numbers in the title, do this in the PR
description below. -->
<!-- ^ For example, instead of "Fixes sagemath#12345" use "Introduce new method
to calculate 1 + 2". -->
<!-- v Describe your changes below in detail. -->
<!-- v Why is this change required? What problem does it solve? -->
<!-- v If this PR resolves an open issue, please link to it here. For
example, "Fixes sagemath#12345". -->



### 📝 Checklist

<!-- Put an `x` in all the boxes that apply. -->

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

### ⌛ Dependencies

<!-- List all open PRs that this PR logically depends on. For example,
-->
<!-- - sagemath#12345: short description why this is a dependency -->
<!-- - sagemath#34567: ... -->
    
URL: sagemath#37808
Reported by: SandwichGouda
Reviewer(s):
vbraun pushed a commit to vbraun/sage that referenced this issue Apr 28, 2024
    
<!-- ^ Please provide a concise and informative title. -->
<!-- ^ Don't put issue numbers in the title, do this in the PR
description below. -->
<!-- ^ For example, instead of "Fixes sagemath#12345" use "Introduce new method
to calculate 1 + 2". -->
<!-- v Describe your changes below in detail. -->
<!-- v Why is this change required? What problem does it solve? -->
<!-- v If this PR resolves an open issue, please link to it here. For
example, "Fixes sagemath#12345". -->

Removing this broken experimental package, as proposed in 2020.
- Closes sagemath#30156

Also removing the module providing outdated "standalone instructions" to
use Gambit.

Keeping the various references to Gambit tagged with `# optional -
gambit`. This will be updated in the follow-up PR"
- sagemath#37809


The motivation for opening this PR to remove `gambit` now (2024) is that
it removes a remaining use of the function `sdh_setup_bdist_wheel` (the
old gambit Python package could not be installed using pip). This a step
for the modernization of the build frontend for Python packages in the
Sage distribution:
- sagemath#35618

### 📝 Checklist

<!-- Put an `x` in all the boxes that apply. -->

- [x] The title is concise and informative.
- [x] 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 and checked the documentation
preview.

### ⌛ Dependencies

<!-- List all open PRs that this PR logically depends on. For example,
-->
<!-- - sagemath#12345: short description why this is a dependency -->
<!-- - sagemath#34567: ... -->
    
URL: sagemath#37810
Reported by: Matthias Köppe
Reviewer(s): John H. Palmieri
vbraun pushed a commit to vbraun/sage that referenced this issue Apr 28, 2024
    
<!-- ^ Please provide a concise and informative title. -->
<!-- ^ Don't put issue numbers in the title, do this in the PR
description below. -->
<!-- ^ For example, instead of "Fixes sagemath#12345" use "Introduce new method
to calculate 1 + 2". -->
<!-- v Describe your changes below in detail. -->
<!-- v Why is this change required? What problem does it solve? -->
<!-- v If this PR resolves an open issue, please link to it here. For
example, "Fixes sagemath#12345". -->



### 📝 Checklist

<!-- Put an `x` in all the boxes that apply. -->

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

### ⌛ Dependencies

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

In Sage 10.4.beta3 we have

```
sage: E.<x,y> = EuclideanSpace()
sage: a = var('a')
sage: v = E.vector_field(x, a*y)
sage: v.divergence().expr()  # OK
a + 1
sage: v.apply_map(lambda t: t.subs(a=-1))
sage: v.display()
x e_x - y e_y
sage: v.divergence().expr()  # wrong!
a + 1
```
The last output should be 0.

This error occurs because the method `_del_derived ` is not invoked to
reset derived quantities relative to the components of a tensor field
that are modified by `apply_map`.  In the above example, the
differentials of the components, which are invoked in computing the
divergence, are cached and not deleted by `apply_map`.
This is corrected here.
    
URL: sagemath#37822
Reported by: Eric Gourgoulhon
Reviewer(s): Matthias Köppe
vbraun pushed a commit to vbraun/sage that referenced this issue Apr 28, 2024
    
<!-- ^ Please provide a concise and informative title. -->
<!-- ^ Don't put issue numbers in the title, do this in the PR
description below. -->
<!-- ^ For example, instead of "Fixes sagemath#12345" use "Introduce new method
to calculate 1 + 2". -->
<!-- v Describe your changes below in detail. -->
<!-- v Why is this change required? What problem does it solve? -->
<!-- v If this PR resolves an open issue, please link to it here. For
example, "Fixes sagemath#12345". -->



### 📝 Checklist

<!-- Put an `x` in all the boxes that apply. -->

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

### ⌛ Dependencies

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

None.

This PR fixes an issue of the method `display_comp` of class
`FreeModuleTensor`, which arises when the LaTeX name of a tensor
contains the symbol `^` or `_`. For instance, in Sage 10.4.beta3, we
have:

```
sage: E.<x,y> = EuclideanSpace()
sage: g = E.metric()
sage: latex(g.inverse().display_comp())
\begin{array}{lcl} g^{-1}_{\phantom{\, x}\phantom{\, x}}^{ \, x \, x } &
= & 1 \\
g^{-1}_{\phantom{\, y}\phantom{\, y}}^{ \, y \, y } & = & 1 \end{array}
```
This is incorrect LaTeX syntax: the symbol  `g` gets two exponent
markers  (`^`). Consequently, in a Jupyter notebook in  `%display latex`
mode, the command `g.inverse().display_comp()` will not generate any
typeset output.

This is fixed here by making the LaTeX output of `display_comp` more
robust: the LaTeX symbol of the tensor (`g^{-1}` in the above example)
is now enclosed between two braces (e.g. `g^{-1}` is changed into
`{g^{-1}}`) before adding indices to it.
    
URL: sagemath#37823
Reported by: Eric Gourgoulhon
Reviewer(s): Matthias Köppe
vbraun pushed a commit to vbraun/sage that referenced this issue Apr 28, 2024
… decorated with basis element names

    
<!-- ^ Please provide a concise and informative title. -->
<!-- ^ Don't put issue numbers in the title, do this in the PR
description below. -->
<!-- ^ For example, instead of "Fixes sagemath#12345" use "Introduce new method
to calculate 1 + 2". -->
<!-- v Describe your changes below in detail. -->
<!-- v Why is this change required? What problem does it solve? -->
<!-- v If this PR resolves an open issue, please link to it here. For
example, "Fixes sagemath#12345". -->

`FiniteRankFreeModuleMorphism` gets a new method `display`, which shows
the matrix with rows and columns decorated with the names of the basis
elements like `\bordermatrix`.

### 📝 Checklist

<!-- Put an `x` in all the boxes that apply. -->

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

### ⌛ Dependencies

<!-- List all open PRs that this PR logically depends on. For example,
-->
<!-- - sagemath#12345: short description why this is a dependency -->
<!-- - sagemath#34567: ... -->
    
URL: sagemath#37825
Reported by: Matthias Köppe
Reviewer(s): Eric Gourgoulhon
vbraun pushed a commit to vbraun/sage that referenced this issue Apr 28, 2024
    
<!-- ^ Please provide a concise and informative title. -->
<!-- ^ Don't put issue numbers in the title, do this in the PR
description below. -->
<!-- ^ For example, instead of "Fixes sagemath#12345" use "Introduce new method
to calculate 1 + 2". -->
<!-- v Describe your changes below in detail. -->
<!-- v Why is this change required? What problem does it solve? -->
<!-- v If this PR resolves an open issue, please link to it here. For
example, "Fixes sagemath#12345". -->

We add more methods that complement the existing methods `det` and
`trace`, by delegating to the existing methods of Sage matrices.

This is parallel to what is done for ModulesWithBasis in:
- sagemath#37731

### 📝 Checklist

<!-- Put an `x` in all the boxes that apply. -->

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

### ⌛ Dependencies

<!-- List all open PRs that this PR logically depends on. For example,
-->
<!-- - sagemath#12345: short description why this is a dependency -->
<!-- - sagemath#34567: ... -->
    
URL: sagemath#37826
Reported by: Matthias Köppe
Reviewer(s): Eric Gourgoulhon, Matthias Köppe, Travis Scrimshaw
vbraun pushed a commit to vbraun/sage that referenced this issue Apr 28, 2024
    
<!-- ^ Please provide a concise and informative title. -->
<!-- ^ Don't put issue numbers in the title, do this in the PR
description below. -->
<!-- ^ For example, instead of "Fixes sagemath#12345" use "Introduce new method
to calculate 1 + 2". -->
<!-- v Describe your changes below in detail. -->
<!-- v Why is this change required? What problem does it solve? -->
<!-- v If this PR resolves an open issue, please link to it here. For
example, "Fixes sagemath#12345". -->



### 📝 Checklist

<!-- Put an `x` in all the boxes that apply. -->

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

### ⌛ Dependencies

<!-- List all open PRs that this PR logically depends on. For example,
-->
<!-- - sagemath#12345: short description why this is a dependency -->
<!-- - sagemath#34567: ... -->
    
URL: sagemath#37827
Reported by: Matthias Köppe
Reviewer(s): Eric Gourgoulhon
vbraun pushed a commit to vbraun/sage that referenced this issue Apr 28, 2024
…t edits

    
<!-- ^ Please provide a concise and informative title. -->
<!-- ^ Don't put issue numbers in the title, do this in the PR
description below. -->
<!-- ^ For example, instead of "Fixes sagemath#12345" use "Introduce new method
to calculate 1 + 2". -->
<!-- v Describe your changes below in detail. -->
<!-- v Why is this change required? What problem does it solve? -->
<!-- v If this PR resolves an open issue, please link to it here. For
example, "Fixes sagemath#12345". -->

Small markup/formatting improvements.

### 📝 Checklist

<!-- Put an `x` in all the boxes that apply. -->

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

### ⌛ Dependencies

<!-- List all open PRs that this PR logically depends on. For example,
-->
<!-- - sagemath#12345: short description why this is a dependency -->
<!-- - sagemath#34567: ... -->
    
URL: sagemath#37829
Reported by: Matthias Köppe
Reviewer(s): Eric Gourgoulhon
vbraun pushed a commit to vbraun/sage that referenced this issue Apr 28, 2024
…stro info

    
<!-- ^ Please provide a concise and informative title. -->
<!-- ^ Don't put issue numbers in the title, do this in the PR
description below. -->
<!-- ^ For example, instead of "Fixes sagemath#12345" use "Introduce new method
to calculate 1 + 2". -->
<!-- v Describe your changes below in detail. -->
<!-- v Why is this change required? What problem does it solve? -->
<!-- v If this PR resolves an open issue, please link to it here. For
example, "Fixes sagemath#12345". -->

- Cherry-picked from sagemath#36181

### 📝 Checklist

<!-- Put an `x` in all the boxes that apply. -->

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

### ⌛ Dependencies

<!-- List all open PRs that this PR logically depends on. For example,
-->
<!-- - sagemath#12345: short description why this is a dependency -->
<!-- - sagemath#34567: ... -->
    
URL: sagemath#37833
Reported by: Matthias Köppe
Reviewer(s): Enrique Manuel Artal Bartolo
vbraun pushed a commit to vbraun/sage that referenced this issue Apr 28, 2024
    
<!-- ^ Please provide a concise and informative title. -->
<!-- ^ Don't put issue numbers in the title, do this in the PR
description below. -->
<!-- ^ For example, instead of "Fixes sagemath#12345" use "Introduce new method
to calculate 1 + 2". -->
<!-- v Describe your changes below in detail. -->
<!-- v Why is this change required? What problem does it solve? -->
<!-- v If this PR resolves an open issue, please link to it here. For
example, "Fixes sagemath#12345". -->

... as suggested by @kwankyu in https://groups.google.com/g/sage-
devel/c/dEa3i2Fn3ZY/m/nzUr2wc_AAAJ

To test: Commands such as `tox -e docker-ubuntu-jammy-minimal --
config.status` should still work.

### 📝 Checklist

<!-- Put an `x` in all the boxes that apply. -->

- [x] The title is concise and informative.
- [x] 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 and checked the documentation
preview.

### ⌛ Dependencies

<!-- List all open PRs that this PR logically depends on. For example,
-->
<!-- - sagemath#12345: short description why this is a dependency -->
<!-- - sagemath#34567: ... -->
    
URL: sagemath#37841
Reported by: Matthias Köppe
Reviewer(s): Kwankyu Lee, Matthias Köppe
vbraun pushed a commit to vbraun/sage that referenced this issue Apr 28, 2024
    
<!-- ^ Please provide a concise and informative title. -->
<!-- ^ Don't put issue numbers in the title, do this in the PR
description below. -->
<!-- ^ For example, instead of "Fixes sagemath#12345" use "Introduce new method
to calculate 1 + 2". -->
<!-- v Describe your changes below in detail. -->
<!-- v Why is this change required? What problem does it solve? -->
<!-- v If this PR resolves an open issue, please link to it here. For
example, "Fixes sagemath#12345". -->

As suspected in
sagemath#37587 (comment)
the issue can be fixed by adding the contraint for the exterior region.

In the differences of this branch just the lines below belong to the
corresponding correction of the algorithm:


```
index 9d988d877c..33d0cae051 100644
--- a/src/sage/knots/link.py
+++ b/src/sage/knots/link.py
@@ -3607,36 +3607,55 @@ class Link(SageObject):
         # such that the resulting regions are in fact closed regions
         # with straight angles, and using the minimal number of bends.
         regions = sorted(self.regions(), key=len)
-        regions = regions[:-1]
         edges = list(set(flatten(pd_code)))
...
+            else:
+                # capacity of exterior region, only sink
+                capacity = len(r) + 4
...
         nregions = []
-        for r in regions:
+        for r in regions[:-1]:  # interior regions
             nregion = []
```

All other changes are intended to improve the readability of the code.
To this end I change and introduce new variable names to make the origin
of the ideas coming from the class [OrthogonalLinkDiagram](https://githu
b.com/3-
manifolds/Spherogram/blob/3062478dd69a52a16e55b967dd46dfe940af9ea7/spher
ogram_src/links/orthogonal.py#L118) of
[Spherogram](https://github.com/3-manifolds/Spherogram) more explicit.
Furthermore I make some codestyle fixes and change the sign of the
capacity such that the sink appears to be positive and thus according to
the usage in Spherogram and the literature cited there.

With this branch the knot reported in the issue is plotted like this:

```
sage: kn = Knots().from_dowker_code([30, 18, 20, 24, 22, 26, 28, 32, 2,
4, 6, 8, 12, 10, 16, 14])
sage: arcs = sorted(kn.arcs())
sage: cols = {tuple(arcs[i]): i for i in range(len(arcs))}
sage: kn.plot(color=cols)
```

### New plot

![kn_new_cols](https://github.com/sagemath/sage/assets/47305845/328ff242
-2a99-48b1-b55a-657f13db0fae)

To make comparison with other results more convenient I used different
colours for the arcs. The corresponding result for `PPL` is this:

```
sage: kn.plot(color=cols, solver='PPL')
```

### New plot with `PPL`

![kn_new_cols_ppl](https://github.com/sagemath/sage/assets/47305845/359b
2404-91a4-44bc-a949-5cc7dec9ba0f)


Without this branch the `PPL` version with colours has been this:

### Old plot with `PPL`

![kn_cols_ppl](https://github.com/sagemath/sage/assets/47305845/5bc9809c
-9aa5-4cb4-b700-7c833e5e598a)

The coloured wrong result has been this:

### Old plot

![kn_cols](https://github.com/sagemath/sage/assets/47305845/513651b5-
8aa7-4394-b5e1-0d6715e7fbe1)


Note, that this branch also changes other plot results in the
documentation. For example:

Previously:

### Old documentation

![grafik](https://github.com/sagemath/sage/assets/47305845/41ef8ad9-
cbbd-42ed-a898-3cd565bab23e)


With this branch:

### New documentation

![grafik](https://github.com/sagemath/sage/assets/47305845/4bcff688-
10e6-4a79-9823-12205d58ac75)




### 📝 Checklist

<!-- Put an `x` in all the boxes that apply. -->

- [x] The title is concise and informative.
- [x] 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 and checked the documentation
preview.

### ⌛ Dependencies

<!-- List all open PRs that this PR logically depends on. For example,
-->
<!-- - sagemath#12345: short description why this is a dependency -->
<!-- - sagemath#34567: ... -->
    
URL: sagemath#37851
Reported by: Sebastian Oehms
Reviewer(s):
vbraun pushed a commit to vbraun/sage that referenced this issue Apr 28, 2024
    
<!-- ^ Please provide a concise and informative title. -->
<!-- ^ Don't put issue numbers in the title, do this in the PR
description below. -->
<!-- ^ For example, instead of "Fixes sagemath#12345" use "Introduce new method
to calculate 1 + 2". -->
<!-- v Describe your changes below in detail. -->
<!-- v Why is this change required? What problem does it solve? -->
<!-- v If this PR resolves an open issue, please link to it here. For
example, "Fixes sagemath#12345". -->

Remove code deprecated in:
- sagemath#31013 (2022)
- sagemath#30747 (2020)
- sagemath#30607 (2020)

### 📝 Checklist

<!-- Put an `x` in all the boxes that apply. -->

- [x] The title is concise and informative.
- [x] 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 and checked the documentation
preview.

### ⌛ Dependencies

<!-- List all open PRs that this PR logically depends on. For example,
-->
<!-- - sagemath#12345: short description why this is a dependency -->
<!-- - sagemath#34567: ... -->
    
URL: sagemath#37855
Reported by: Matthias Köppe
Reviewer(s): François Bissey, Matthias Köppe
vbraun pushed a commit to vbraun/sage that referenced this issue Apr 28, 2024
 (2022)

    
<!-- ^ Please provide a concise and informative title. -->
<!-- ^ Don't put issue numbers in the title, do this in the PR
description below. -->
<!-- ^ For example, instead of "Fixes sagemath#12345" use "Introduce new method
to calculate 1 + 2". -->
<!-- v Describe your changes below in detail. -->
<!-- v Why is this change required? What problem does it solve? -->
<!-- v If this PR resolves an open issue, please link to it here. For
example, "Fixes sagemath#12345". -->

Deprecated in:
- sagemath#30207 (2022)

### 📝 Checklist

<!-- Put an `x` in all the boxes that apply. -->

- [x] The title is concise and informative.
- [ ] 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 and checked the documentation
preview.

### ⌛ Dependencies

<!-- List all open PRs that this PR logically depends on. For example,
-->
<!-- - sagemath#12345: short description why this is a dependency -->
<!-- - sagemath#34567: ... -->
    
URL: sagemath#37856
Reported by: Matthias Köppe
Reviewer(s):
vbraun pushed a commit to vbraun/sage that referenced this issue Apr 28, 2024
    
<!-- ^ Please provide a concise and informative title. -->
<!-- ^ Don't put issue numbers in the title, do this in the PR
description below. -->
<!-- ^ For example, instead of "Fixes sagemath#12345" use "Introduce new method
to calculate 1 + 2". -->
<!-- v Describe your changes below in detail. -->
<!-- v Why is this change required? What problem does it solve? -->
<!-- v If this PR resolves an open issue, please link to it here. For
example, "Fixes sagemath#12345". -->

Deprecated in
- sagemath#31591 (2021)
- sagemath#32234 (2021)

### 📝 Checklist

<!-- Put an `x` in all the boxes that apply. -->

- [x] The title is concise and informative.
- [ ] 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 and checked the documentation
preview.

### ⌛ Dependencies

<!-- List all open PRs that this PR logically depends on. For example,
-->
<!-- - sagemath#12345: short description why this is a dependency -->
<!-- - sagemath#34567: ... -->
    
URL: sagemath#37870
Reported by: Matthias Köppe
Reviewer(s): Michael Orlitzky
vbraun pushed a commit to vbraun/sage that referenced this issue Apr 28, 2024
…nterface

    
<!-- ^ Please provide a concise and informative title. -->
<!-- ^ Don't put issue numbers in the title, do this in the PR
description below. -->
<!-- ^ For example, instead of "Fixes sagemath#12345" use "Introduce new method
to calculate 1 + 2". -->
<!-- v Describe your changes below in detail. -->
<!-- v Why is this change required? What problem does it solve? -->
<!-- v If this PR resolves an open issue, please link to it here. For
example, "Fixes sagemath#12345". -->

- Deprecated in sagemath#33777 (2022).

### 📝 Checklist

<!-- Put an `x` in all the boxes that apply. -->

- [x] The title is concise and informative.
- [ ] 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 and checked the documentation
preview.

### ⌛ Dependencies

<!-- List all open PRs that this PR logically depends on. For example,
-->
<!-- - sagemath#12345: short description why this is a dependency -->
<!-- - sagemath#34567: ... -->
    
URL: sagemath#37872
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
Projects
None yet
Development

No branches or pull requests

3 participants