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

CombinatorialPolyhedron: Remove empty folder #28757

Closed
kliem opened this issue Nov 18, 2019 · 23 comments
Closed

CombinatorialPolyhedron: Remove empty folder #28757

kliem opened this issue Nov 18, 2019 · 23 comments

Comments

@kliem
Copy link
Contributor

kliem commented Nov 18, 2019

In the folder src/sage/geometry/polyhedron/combinatorial_polyhedron there is an empty folder that keeps appearing.

This is because bit_vector_operations was not properly defined in module_list.py.

We fix this by including the functions needed directly in each file with cdef extern from "bit_vector_operations.cc" and removing the module sage/geometry/polyhedron/combinatorial_polyhedron/bit_vector_operations.cc.

CC: @jplab @LaisRast

Component: geometry

Keywords: combinatorial polyhedron

Author: Jonathan Kliem

Branch/Commit: c698941

Reviewer: Jean-Philippe Labbé

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

@kliem kliem added this to the sage-9.0 milestone Nov 18, 2019
@kliem
Copy link
Contributor Author

kliem commented Nov 18, 2019

New commits:

3cf386cstop empty folder from appearing

@kliem
Copy link
Contributor Author

kliem commented Nov 18, 2019

Branch: public/28757

@kliem
Copy link
Contributor Author

kliem commented Nov 18, 2019

Commit: 3cf386c

@embray
Copy link
Contributor

embray commented Jan 6, 2020

comment:2

Ticket retargeted after milestone closed

@embray embray modified the milestones: sage-9.0, sage-9.1 Jan 6, 2020
@kliem
Copy link
Contributor Author

kliem commented Jan 31, 2020

comment:3

This is a new approach. I remove the module bit_vector_operations completely. Instead each file just includes whatever it needs from bit_vector_operations.cc.


New commits:

ae89a8bremove empty folder in combinatorial polyhedron

@kliem
Copy link
Contributor Author

kliem commented Jan 31, 2020

Changed commit from 3cf386c to ae89a8b

@kliem
Copy link
Contributor Author

kliem commented Jan 31, 2020

Changed branch from public/28757 to public/28757-new

@kliem
Copy link
Contributor Author

kliem commented Mar 19, 2020

comment:4

Apply failed.

@kliem
Copy link
Contributor Author

kliem commented Mar 19, 2020

New commits:

a584181remove empty folder in combinatorial polyhedron

@kliem
Copy link
Contributor Author

kliem commented Mar 19, 2020

Changed branch from public/28757-new to public/28757-reb

@kliem
Copy link
Contributor Author

kliem commented Mar 19, 2020

Changed commit from ae89a8b to a584181

@jplab
Copy link

jplab commented Apr 19, 2020

comment:6

Seems still to cause a merge conflict...

@kliem
Copy link
Contributor Author

kliem commented Apr 19, 2020

comment:7

Not still. Again. The annoying thing about this approach is that I'm very likely to conflict.

@kliem
Copy link
Contributor Author

kliem commented Apr 19, 2020

Changed commit from a584181 to c698941

@kliem
Copy link
Contributor Author

kliem commented Apr 19, 2020

New commits:

c698941remove empty folder in combinatorial polyhedron

@kliem
Copy link
Contributor Author

kliem commented Apr 19, 2020

Changed branch from public/28757-reb to public/28757-reb2

@jplab
Copy link

jplab commented Apr 20, 2020

comment:10

Could you update the description of the ticket to also mention the new functions that are added?

Somehow, this ticket does more than changing 2-3 lines... Perhaps also change the title of the ticket. This will help in the future to traceback if necessary...

@jplab
Copy link

jplab commented Apr 20, 2020

Reviewer: Jean-Philippe Labbé

@kliem
Copy link
Contributor Author

kliem commented Apr 20, 2020

comment:11

There are no new functions. It's just that we know grep them directly from the .cc file:

-from .bit_vector_operations cimport intersection, bit_rep_to_coatom_rep
+
+cdef extern from "bit_vector_operations.cc":
...

@kliem

This comment has been minimized.

@jplab
Copy link

jplab commented Apr 20, 2020

comment:13

Got it. Thanks for clarifying... it has been a while...

@kliem
Copy link
Contributor Author

kliem commented Apr 20, 2020

comment:14

Thanks.

@vbraun
Copy link
Member

vbraun commented Apr 23, 2020

Changed branch from public/28757-reb2 to c698941

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