SeaGriff / Graph-Orientations-and-Divisors

MIT License
7 stars 1 forks source link

deprecation warnings related to `sort` #3

Closed dimpase closed 1 year ago

dimpase commented 1 year ago

here with Sage 9.8.beta3:

sage: load('example_constructions.sage')
sage: ex1()
/home/dimpase/work/software/Graph-Orientations-and-Divisors/CycleCocycleSystem.py:296: DeprecationWarning: parameter 'sort' will be set to False by default in the future
See https://trac.sagemath.org/27408 for details.
  if len(set(data.edge_labels())) < len(data.edges()):
/home/dimpase/work/software/Graph-Orientations-and-Divisors/CycleCocycleSystem.py:306: DeprecationWarning: parameter 'sort' will be set to False by default in the future
See http://trac.sagemath.org/22349 for details.
  self._bi = DiGraph([self.vertices(), bi],
/home/dimpase/work/software/Graph-Orientations-and-Divisors/CycleCocycleSystem.py:309: DeprecationWarning: parameter 'sort' will be set to False by default in the future
See http://trac.sagemath.org/22349 for details.
  self._unori = DiGraph([self.vertices(), unori],
/home/dimpase/work/software/Graph-Orientations-and-Divisors/CycleCocycleSystem.py:29: DeprecationWarning: parameter 'sort' will be set to False by default in the future
See https://trac.sagemath.org/27408 for details.
  if len(set(data.edge_labels())) < len(data.edges()):
/home/dimpase/work/software/Graph-Orientations-and-Divisors/CycleCocycleSystem.py:44: DeprecationWarning: parameter 'sort' will be set to False by default in the future
See https://trac.sagemath.org/27408 for details.
  elif len(set(base_orientation.edge_labels())) < len(base_orientation.edges()):
/home/dimpase/work/software/Graph-Orientations-and-Divisors/newmethods.py:146: DeprecationWarning: parameter 'sort' will be set to False by default in the future
See https://trac.sagemath.org/27408 for details.
  return {e[2]: e for e in G.edges()}
/home/dimpase/work/software/Graph-Orientations-and-Divisors/CycleCocycleSystem.py:276: DeprecationWarning: parameter 'sort' will be set to False by default in the future
See https://trac.sagemath.org/27408 for details.
  return edge_signs(self._base_orientation.edges(), U)
/home/dimpase/work/software/Graph-Orientations-and-Divisors/CycleCocycleSystem.py:760: DeprecationWarning: parameter 'sort' will be set to False by default in the future
See https://trac.sagemath.org/27408 for details.
  edge_set = set({e for e in CCS.edges() if e[2] in C})
/home/dimpase/work/software/Graph-Orientations-and-Divisors/CycleCocycleSystem.py:60: DeprecationWarning: parameter 'sort' will be set to False by default in the future
See http://trac.sagemath.org/22349 for details.
  len(self.vertices()), len(self.edges())))
/home/dimpase/work/software/Graph-Orientations-and-Divisors/CycleCocycleSystem.py:60: DeprecationWarning: parameter 'sort' will be set to False by default in the future
See https://trac.sagemath.org/27408 for details.
  len(self.vertices()), len(self.edges())))
(A graph with a cycle-cocycle reversal system, on 5 vertices and with 7 edges,
 A graph with a cycle-cocycle reversal system, on 5 vertices and with 7 edges,
 A morphism between cycle cocycle systems.)
dimpase commented 1 year ago

@dcoudert - do you understand why these deprecations are popping up, despite sort= not being used anywhere in the code here? (By the way, time to remove them from Sage, they have been on for long enough)

dcoudert commented 1 year ago

The current default behavior is to raise a deprecation warning when parameter sort is not specified. This is ticket https://trac.sagemath.org/ticket/22349 which has been merged only 3 months ago.

dimpase commented 1 year ago

however, https://trac.sagemath.org/27408 has been merged in Sage 9.0, and it still pops up.

@SeaGriff could hopefully tell more about the need for an explicit ordering of edges and vertices in the graphs there.

dcoudert commented 1 year ago

For some reason, the deprecation warning introduced in https://trac.sagemath.org/27408 was not visible. Do you think we should proceed with the removal of the warning ?

@SeaGriff in your code, use G.order() instead of len(G.vertices()). It avoids to first create the list of vertices and then get the length of this list. The graph knows its number of vertices. Similarly, use G.size() instead of len(G.edges()).

dimpase commented 1 year ago

the deprecation warning introduced in https://trac.sagemath.org/27408 was not visible

as we see here, it is visible in this case. So I don't quite get what you mean by "was not visible".

dcoudert commented 1 year ago

The deprecation warning had no effect before https://trac.sagemath.org/ticket/22349

dimpase commented 1 year ago

The deprecation warning had no effect before https://trac.sagemath.org/ticket/22349

oops, I see. OK, then indeed we'd wait till late summer 2023 with removal, I suppose.

dcoudert commented 1 year ago

At least the beginning of next summer.

SeaGriff commented 1 year ago

I can't recall any reason that information needs to be sorted, and I can't find any in the code, and everything seems to work just as well with the sort flag set to False. This and the optimization @dcoudert mentioned are now on the main branch. thank you!!!

dimpase commented 1 year ago

the next step would be to add Sage-style doctests to all public functions in .py files.

See

https://doc.sagemath.org/html/en/developer/doctesting.html#beyond-the-sage-library

on how to run such tests, once they are there.

dimpase commented 1 year ago

ok, done