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

Method to get cells in horizontal/vertical border strip #34345

Closed
trevorkarn opened this issue Aug 11, 2022 · 13 comments
Closed

Method to get cells in horizontal/vertical border strip #34345

trevorkarn opened this issue Aug 11, 2022 · 13 comments

Comments

@trevorkarn
Copy link
Contributor

The methods Partition.add_*_border_strip where * is horizontal or vertical return new partitions. This ticket proposes a method that returns the indices of the cells of these new partitions which are not in the original partition.

Depends on #34339

CC: @tscrim @trevorkarn

Component: combinatorics

Keywords: gsoc2022 partition cell border-strip

Author: Trevor K. Karn

Branch/Commit: 218b548

Reviewer: Travis Scrimshaw

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

@trevorkarn
Copy link
Contributor Author

Branch: u/tkarn/strip-cells-34345

@trevorkarn
Copy link
Contributor Author

New commits:

6ccd9e4Optimization of add_*_border_strip() and doctest update.
69d1ea8Add horizontal cells

@trevorkarn
Copy link
Contributor Author

Commit: 69d1ea8

@trevorkarn
Copy link
Contributor Author

Dependencies: #34339

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Aug 12, 2022

Changed commit from 69d1ea8 to 3d7a37b

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Aug 12, 2022

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

3d7a37bAdd vertical strips

@tscrim
Copy link
Collaborator

tscrim commented Aug 19, 2022

comment:5

For these methods, since we are adding them, it would be better if they were generators that iterated through the different strips. (I wanted to do this for the other methods, but I didn't want to deal with the backwards incompatibility.) So instead of res.append(foo), it would be yield foo.

Why the change in order (horizontal then vertical)? It would be better to be consistent.

In the vertical strip, you can move the creation of tmp outside of the for loop.

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Aug 30, 2022

Changed commit from 3d7a37b to 218b548

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Aug 30, 2022

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

218b548Make a generator, reorder, and move tmp variable

@trevorkarn
Copy link
Contributor Author

comment:7

Should be fixed.

@tscrim
Copy link
Collaborator

tscrim commented Aug 30, 2022

comment:8

Thanks. LGTM.

@tscrim
Copy link
Collaborator

tscrim commented Aug 30, 2022

Reviewer: Travis Scrimshaw

@vbraun
Copy link
Member

vbraun commented Sep 22, 2022

Changed branch from u/tkarn/strip-cells-34345 to 218b548

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

4 participants