Opened 3 years ago

Closed 2 years ago

#28757 closed enhancement (fixed)

CombinatorialPolyhedron: Remove empty folder

Reported by: gh-kliem Owned by:
Priority: major Milestone: sage-9.1
Component: geometry Keywords: combinatorial polyhedron
Cc: jipilab, gh-LaisRast Merged in:
Authors: Jonathan Kliem Reviewers: Jean-Philippe Labbé
Report Upstream: N/A Work issues:
Branch: c698941 (Commits, GitHub, GitLab) Commit: c698941441fe9be862bd5740f52dda9b900b972a
Dependencies: Stopgaps:

Status badges

Description (last modified by gh-kliem)

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.

Change History (15)

comment:1 Changed 3 years ago by gh-kliem

  • Branch set to public/28757
  • Commit set to 3cf386ce513b4123dcc0f961f8e5848b1030a3c6
  • Status changed from new to needs_review

New commits:

​3cf386cstop empty folder from appearing

comment:2 Changed 3 years ago by embray

  • Milestone changed from sage-9.0 to sage-9.1

Ticket retargeted after milestone closed

comment:3 Changed 3 years ago by gh-kliem

  • Branch changed from public/28757 to public/28757-new
  • Commit changed from 3cf386ce513b4123dcc0f961f8e5848b1030a3c6 to ae89a8b44ef95e3e8d94d7bc8edb69cc33a119f1

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

comment:4 Changed 2 years ago by gh-kliem

  • Status changed from needs_review to needs_work

Apply failed.

comment:5 Changed 2 years ago by gh-kliem

  • Branch changed from public/28757-new to public/28757-reb
  • Commit changed from ae89a8b44ef95e3e8d94d7bc8edb69cc33a119f1 to a584181495a1d6ab6b8be917cc5c816e4c6b06c5
  • Status changed from needs_work to needs_review

New commits:

​a584181remove empty folder in combinatorial polyhedron

comment:6 Changed 2 years ago by jipilab

  • Status changed from needs_review to needs_work

Seems still to cause a merge conflict...

comment:7 Changed 2 years ago by gh-kliem

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

comment:8 Changed 2 years ago by gh-kliem

  • Branch changed from public/28757-reb to public/28757-reb2
  • Commit changed from a584181495a1d6ab6b8be917cc5c816e4c6b06c5 to c698941441fe9be862bd5740f52dda9b900b972a

New commits:

​c698941remove empty folder in combinatorial polyhedron

comment:9 Changed 2 years ago by gh-kliem

  • Status changed from needs_work to needs_review

comment:10 Changed 2 years ago by jipilab

  • Reviewers set to Jean-Philippe Labbé
  • Status changed from needs_review to needs_work

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

comment:11 Changed 2 years ago by gh-kliem

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":
...

comment:12 Changed 2 years ago by gh-kliem

  • Description modified (diff)
  • Status changed from needs_work to needs_review

comment:13 Changed 2 years ago by jipilab

  • Status changed from needs_review to positive_review

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

comment:14 Changed 2 years ago by gh-kliem

Thanks.

comment:15 Changed 2 years ago by vbraun

  • Branch changed from public/28757-reb2 to c698941441fe9be862bd5740f52dda9b900b972a
  • Resolution set to fixed
  • Status changed from positive_review to closed
Note: See TracTickets for help on using tickets.