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

Numerical/scipbackend faster copy method #24

Conversation

mantepse
Copy link

Resolve trivial merge conflicts with develop.

mkoeppe and others added 30 commits January 31, 2024 19:06
…platforms; do not invite users to port to random unsupported platforms
Also remove accidental change to documentation of unrelated function from previous commit
Co-authored-by: Lorenz Panny <84067835+yyyyx4@users.noreply.github.com>
Release Manager and others added 27 commits February 11, 2024 17:02
    
Added the possibility to provide a rational slope parameter m in the
GeneralizedTamariLattice method.
The lattice property is no longer conjectural so the check parameter is
not necessary anymore.
Also updated the references.

<!-- 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.
- [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 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#37209
Reported by: Cchenevi
Reviewer(s): Frédéric Chapoton
    
We unify the definitions as follows:

* `substitute` is a method in `Element` which calls `self.subs` and
passes all arguments along, and not defined anywhere else.
* `subs` is the method that should be overwritten in the various element
classes.

Dependencies:

sagemath#37143
    
URL: sagemath#37210
Reported by: Martin Rubey
Reviewer(s): Frédéric Chapoton
    
For the installation guide there was an erroneous bullet point which I
have removed.
    
URL: sagemath#37215
Reported by: Giacomo Pope
Reviewer(s): Kwankyu Lee
    
The nesting function in combinat.ncsym.ncsym.py does not operate as
described in its documentation.  As intended, it should compare all arcs
of two set partitions and count the number of times the second arc nests
in the first.  As written, there was a break statement caused many
(necessary) comparisons to be skipped, particularly for set partitions
with large blocks.

I have re-written the function so that all necessary comparisons are
made.  I have also added an example in the docstring that was computed
incorrectly by the old code, but is corrected by my fix.

This bug is not a previously known issue, and I have also confirmed with
the original author (@tscrim ) that no one else is currently working on
a fix.

### 📝 Checklist

- [x] The title is concise, informative, and self-explanatory.
- [x] The description explains in detail what this PR is about.

### ⌛ Dependencies

No dependencies
    
URL: sagemath#37218
Reported by: lucasgagnon
Reviewer(s): Travis Scrimshaw
…ages: ..." to be set

    
<!-- ^^^^^
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 -->

The workflow is already conditionalized to only run when changes to
files in `build/pkgs/` or `pkgs/` are made, which is good enough.

<!-- 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.
- [ ] 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#37222
Reported by: Matthias Köppe
Reviewer(s):
    
This PR removes the depreciated invariants from sagemath#28064 (2019). It's been
long enough and we should declutter the tab-autocomplete from these very
specific functions.

As an aside, I'm not sure why we have:

```py
from . import monsky_washnitzer
```

Instead of importing objects from this module, but I have not worked
with this myself so am loathed to modify this without further input from
the community.
    
URL: sagemath#37223
Reported by: Giacomo Pope
Reviewer(s): Frédéric Chapoton
    
<!-- ^^^^^
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 -->

This PR partially addresses
sagemath#22165. It removes one (in
`splitting_field`) of the two uses of the Pari deprecated function
`polred`. The other one is less trivial to remove and should probably be
discussed in the issue.

<!-- 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 linked a relevant issue or discussion.
- [ ] I have created tests covering the changes. [the code is meant to
be functionally equivalent]
- [x] I have updated the documentation accordingly.

### ⌛ Dependencies
None
<!-- 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#37225
Reported by: AurelPage
Reviewer(s): Vincent Delecroix
…hen S is empty

    
<!-- ^^^^^
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 -->

This PR fixes sagemath#36386, at least
for unit group computations (S-units will still suffer from the problem,
or when the units are even larger). This PR does **not** address
sagemath#31754, which would be a better
(but more complicated) solution for the problem.

<!-- 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 linked a relevant issue or discussion.
- [x] I have created tests covering the changes.
- [ ] I have updated the documentation accordingly.

### ⌛ Dependencies
None
<!-- 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#37226
Reported by: AurelPage
Reviewer(s): AurelPage, Vincent Delecroix
    
This fixes all remaining pycodestyle E221 warnings in py files and
activate this check in the pycodestyle-minimal linter.

This is about `E221 multiple spaces before operator`

### 📝 Checklist

- [x] The title is concise, informative, and self-explanatory.
- [x] The description explains in detail what this PR is about.
    
URL: sagemath#37228
Reported by: Frédéric Chapoton
Reviewer(s): Travis Scrimshaw
    
this fixes various pycodestyle warnings in coding, in particular E275

`E275 missing whitespace after keyword`

### 📝 Checklist

- [x] The title is concise, informative, and self-explanatory.
- [x] The description explains in detail what this PR is about.
    
URL: sagemath#37229
Reported by: Frédéric Chapoton
Reviewer(s): David Coudert
    
a few details as suggested by `cython-lint` and `pycodestyle` in
`libs/ecl.pyx`

### 📝 Checklist

- [x] The title is concise, informative, and self-explanatory.
- [x] The description explains in detail what this PR is about.
    
URL: sagemath#37230
Reported by: Frédéric Chapoton
Reviewer(s): David Coudert
    
this is removing many calls to isinstance of old-style parent-like
classes

one should use instead the corresponding containment in categories

the case of `Finitefield` is kept for another time

### 📝 Checklist

- [x] The title is concise, informative, and self-explanatory.
- [x] The description explains in detail what this PR is about.
    
URL: sagemath#37232
Reported by: Frédéric Chapoton
Reviewer(s): Martin Rubey, Matthias Köppe
…documentation

    
The possibility to pass discriminants to the constructor instead of
`BQFClassGroup` objects was present for an earlier version of sagemath#37074,
but it was removed during review. However, it seems I forgot to update
the docstring accordingly.
    
URL: sagemath#37233
Reported by: Lorenz Panny
Reviewer(s): Travis Scrimshaw
…om ring.pyx

    
This is creating a `DedekindDomains` category and moving the associated
code from `ring.pyx` to the category.

### 📝 Checklist

- [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.
- [x] I have updated the documentation accordingly.
    
URL: sagemath#37234
Reported by: Frédéric Chapoton
Reviewer(s): Travis Scrimshaw
    
as this appeared to be useful and missing during #sd125

### 📝 Checklist

- [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.
- [x] I have created tests covering the changes.
- [x] I have updated the documentation accordingly.
    
URL: sagemath#37240
Reported by: Frédéric Chapoton
Reviewer(s): Travis Scrimshaw
    
namely

- use ruff to fix some containement tests

- add links to some error codes in the doc

### 📝 Checklist

- [x] The title is concise, informative, and self-explanatory.
- [x] The description explains in detail what this PR is about.
    
URL: sagemath#37245
Reported by: Frédéric Chapoton
Reviewer(s): David Coudert, Frédéric Chapoton
    
namely

- use ruff to fix some containement tests

- add links to some error codes in the doc

- plus some http to https

### 📝 Checklist

- [x] The title is concise, informative, and self-explanatory.
- [x] The description explains in detail what this PR is about.
    
URL: sagemath#37246
Reported by: Frédéric Chapoton
Reviewer(s): Matthias Köppe
    
<!-- ^^^^^
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 -->
See easimon/maximize-build-space#39

<!-- 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.
- [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
- 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#37252
Reported by: Matthias Köppe
Reviewer(s): Sebastian Oehms
    
Just refreshing the modified file, pep8 for code, details in doc, etc

### 📝 Checklist

- [x] The title is concise, informative, and self-explanatory.
- [x] The description explains in detail what this PR is about.
    
URL: sagemath#37256
Reported by: Frédéric Chapoton
Reviewer(s): Matthias Köppe
    
This is adding type annotations to many `gens` methods.

This will help to make them all return `tuple` later.

### 📝 Checklist

- [x] The title is concise, informative, and self-explanatory.
- [x] The description explains in detail what this PR is about.
    
URL: sagemath#37258
Reported by: Frédéric Chapoton
Reviewer(s): Matthias Köppe
    
Add "needs sphinx" tags to some tests in `src/sage_docbuild` so there
is no failure when sphinx is not installed in the system.

See:
sagemath#37263 (comment)

### 📝 Checklist

- [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.
    
URL: sagemath#37269
Reported by: Gonzalo Tornaría
Reviewer(s):
    
This is a minimal PR to allow building with python 3.12, given the
circumstances surrounding sagemath#36181.

I've tested this works with system python 3.12 by:
```
./bootstrap
./configure --disable-doc --disable-editable --enable-system-site-
packages
env 'MAKE=make -j36' make
./sage -tp 36 --all
./sage -tp 36 --all --long
```

It only gave the expected failures in
`src/sage/matroids/database_collections.py` from sagemath#37140.

Since scipy 1.12 works ok after sagemath#37123 and it's just a one-liner to
change the required version, I included it here.

No attempt is made at upgrading anything in sage-the-distro.

### 📝 Checklist

- [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.

EDIT: rebased on top of sagemath#36983 to address reviewer suggestion.
    
URL: sagemath#37270
Reported by: Gonzalo Tornaría
Reviewer(s): Aliaksei Urbanski, Gonzalo Tornaría, Tobias Diez
…`nauty_directg`

    
Since version 2.8, nauty's `directg` has a new option `-a`  to generate
only acyclic orientations. This option was already working but not
documented. This PR documents this functionality.

This partially answer issue sagemath#37253 and is related to PR sagemath#35075.

### 📝 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 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
- 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#37272
Reported by: David Coudert
Reviewer(s): Martin Rubey
    
<!-- ^^^^^
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 -->
These are some of the last remaining doctest warnings as of 10.3.beta7,
as seen e.g. in https://github.com/sagemath/sage/pull/37250/files
"Unchanged files with check annotations".

The ones not fixed here will need a solution for:
- sagemath#36099

<!-- 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#37279
Reported by: Matthias Köppe
Reviewer(s): David Coudert
…dist

The implementation of sage.misc.package_dir.read_distributions uses the
function `Cython.Utils.open_source_file()` to open a file instead of
`io.open()`. This function was introduced in sagemath#29701 (b582789).

The only difference between `open_source_file()` and `io.open()` is that
the former will open the file as binary to check the encoding. But all
our files should be utf-8 so we don't need this trick.

In case I’m mistaken and there is a reason this trick is really
necessary (@mkoeppe?) it seems better to include a version of
open_source_file(), for two main reasons: (a) we don’t pull the whole of
Cython just for a trivial function; (b) changes in Cython don’t affect
sagemath (this is not a documented function of Cython, afaict)

### 📝 Checklist

- [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.

URL: sagemath#37290
Reported by: Gonzalo Tornaría
Reviewer(s): Gonzalo Tornaría, Matthias Köppe
@mkoeppe
Copy link
Owner

mkoeppe commented Feb 17, 2024

@mkoeppe mkoeppe merged commit e716fb4 into mkoeppe:t/34890/scipbackend__faster_copy_method Feb 18, 2024
1 check passed
mkoeppe pushed a commit that referenced this pull request Apr 27, 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:
```
#24 4091.8 /sage/build/bin/sage-logger: line 66: /usr/bin/time: No such
file or directory
#24 4091.8 [sagemath_doc_html-none] installing. Log file:
/sage/logs/pkgs/sagemath_doc_html-none.log
#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
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet