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

Deprecate global imports of CombinatorialObject, CombinatorialClass, MapCombinatorialClass #33384

Closed
mkoeppe opened this issue Feb 19, 2022 · 10 comments · Fixed by #37722
Closed

Comments

@mkoeppe
Copy link
Contributor

mkoeppe commented Feb 19, 2022

Part of #12913 (Meta-ticket: Deprecate CombinatorialClass in favor of the EnumeratedSet's categories)

CC: @tscrim @fchapoton

Component: combinatorics

Author: Matthias Koeppe

Branch/Commit: u/mkoeppe/deprecate_global_imports_of_combinatorialobject__combinatorialclass__mapcombinatorialclass @ 1d3a09b

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

@mkoeppe mkoeppe added this to the sage-9.6 milestone Feb 19, 2022
@mkoeppe
Copy link
Contributor Author

mkoeppe commented Feb 19, 2022

@mkoeppe
Copy link
Contributor Author

mkoeppe commented Feb 19, 2022

New commits:

231fe03src/sage/combinat/all.py: Deprecate the global imports of CombinatorialObject, CombinatorialClass, MapCombinatorialClass
81fcc92src/doc/en/developer/coding_basics.rst: Replace use of CombinatorialObject in doctest

@mkoeppe
Copy link
Contributor Author

mkoeppe commented Feb 19, 2022

Commit: 81fcc92

@mkoeppe
Copy link
Contributor Author

mkoeppe commented Feb 19, 2022

Author: Matthias Koeppe

@mkoeppe

This comment has been minimized.

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Feb 19, 2022

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

d4810bfsrc/sage/misc/persist.pyx: Import CombinatorialObject in doctest here
ad521cfsrc/sage/combinat/combinat.py: Add deprecation warning in doctest output
1d3a09bsrc/sage/combinat/combinat.py: Explicitly import CombinatorialClass in some doctests

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Feb 19, 2022

Changed commit from 81fcc92 to 1d3a09b

@mkoeppe
Copy link
Contributor Author

mkoeppe commented Feb 19, 2022

comment:4

The deprecation via lazy imports unfortunately does not work very well:

sage -t --random-seed=332440843511251861710123815794934176969 src/sage/combinat/combinat.py
**********************************************************************
File "src/sage/combinat/combinat.py", line 1198, in sage.combinat.combinat.CombinatorialObject.__eq__
Failed example:
    class Bar(Element, CombinatorialObject):
        def __init__(self, l):
            CombinatorialObject.__init__(self, l)
Exception raised:
    Traceback (most recent call last):
      File "/Users/mkoeppe/s/sage/sage-rebasing/worktree-gcc11/local/var/lib/sage/venv-python3.9/lib/python3.9/site-packages/sage/doctest/forker.py", line 694, in _run
        self.compile_and_execute(example, compiler, test.globs)
      File "/Users/mkoeppe/s/sage/sage-rebasing/worktree-gcc11/local/var/lib/sage/venv-python3.9/lib/python3.9/site-packages/sage/doctest/forker.py", line 1088, in compile_and_execute
        exec(compiled, globs)
      File "<doctest sage.combinat.combinat.CombinatorialObject.__eq__[7]>", line 1, in <module>
        class Bar(Element, CombinatorialObject):
    TypeError: metaclass conflict: the metaclass of a derived class must be a (non-strict) subclass of the metaclasses of all its bases

@tscrim
Copy link
Collaborator

tscrim commented Feb 22, 2022

comment:6

Hmm...that is a bit strange. Perhaps something is not being resolved correctly? I don't know from a quick look what is going on. However, +1 for doing this ticket.

@mkoeppe mkoeppe modified the milestones: sage-9.6, sage-9.7 Apr 7, 2022
@mkoeppe mkoeppe modified the milestones: sage-9.7, sage-9.8 Aug 31, 2022
@mkoeppe mkoeppe modified the milestones: sage-9.8, sage-9.9 Jan 7, 2023
@mkoeppe mkoeppe modified the milestones: sage-10.0, sage-10.1 Apr 30, 2023
@mkoeppe mkoeppe removed this from the sage-10.1 milestone Aug 7, 2023
@mkoeppe
Copy link
Contributor Author

mkoeppe commented Apr 2, 2024

Superseded by #37722

vbraun pushed a commit to vbraun/sage that referenced this issue Apr 20, 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". -->

- Fixes sagemath#36133
- Fixes sagemath#33384
- Fixes sagemath#19474
- Fixes sagemath#12913

### 📝 Checklist

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

- [ ] 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#37722
Reported by: Matthias Köppe
Reviewer(s): Martin Rubey, Matthias Köppe, Travis Scrimshaw
@vbraun vbraun closed this as completed in 46b7ec2 May 2, 2024
@mkoeppe mkoeppe added this to the sage-10.4 milestone May 2, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants