Diagram Layout #1429

Merged
merged 87 commits into from Aug 7, 2012

Projects

None yet

6 participants

@scolobb

This pull request adds the code which lays out the objects of a diagram, depending how they are connected with morphisms and some other parameters.

Some tests for this pull request still fail half of the time, because of the issue with FiniteSet sort keys. I do submit this pull request though so that people can start reviewing it, while I'll be fixing the FiniteSet issue in parallel.

@scolobb scolobb referenced this pull request Jul 18, 2012
Merged

Diagram Drawing #1430

@travisbot

This pull request fails (merged 698063bc into be3da02).

@Krastanov
SymPy member

SymPy Bot Summary: 🔴 There were test failures.

@scolobb: Please fix the test failures.

Test command: setup.py test
master hash: be3da02
branch hash: 698063bce9b6003a6839e9ada4692093b80478df

Interpreter 1: ✳️ All tests have passed.

Interpreter: /usr/local/bin/python2.5 (2.5.6-final-0)
Architecture: Linux (64-bit)
Cache: yes

Test results html report: http://reviews.sympy.org/report/agZzeW1weTNyDAsSBFRhc2sY9sAhDA

Interpreter 2: 🔴 There were test failures.

Interpreter: /usr/bin/python2.7 (2.7.3-candidate-2)
Architecture: Linux (64-bit)
Cache: yes

Test results html report: http://reviews.sympy.org/report/agZzeW1weTNyDAsSBFRhc2sY6PIgDA

Interpreter 3: 🔴 There were test failures.

Interpreter: /usr/bin/python3.2 (3.2.3-candidate-2)
Architecture: Linux (64-bit)
Cache: yes

Test results html report: http://reviews.sympy.org/report/agZzeW1weTNyDAsSBFRhc2sY5_IgDA

Build HTML Docs: 🔴 There were test failures.

Docs build command: make html-errors
Sphinx version: 1.1.3

Test results html report: http://reviews.sympy.org/report/agZzeW1weTNyDAsSBFRhc2sYprEhDA

Automatic review by SymPy Bot.

@ness01

Ok. I have been sequentially through all the commits. Almost everything is fine, except:

  • handling of cases where the algorithm does not succeed
  • DiagramGrid has absolutely no pretty printing
  • some nitpicking

Let me know when these are fixed, and I will look through the stuff again.

@ness01 ness01 and 1 other commented on an outdated diff Jul 22, 2012
sympy/categories/diagram_drawing.py
+ >>> h = NamedMorphism(D, A, "h")
+ >>> k = NamedMorphism(D, B, "k")
+ >>> diagram = Diagram([f, g, h, k])
+
+ Lay it out with generic layout:
+
+ >>> grid = DiagramGrid(diagram)
+ >>> [grid[0, j] for j in xrange(grid.width)]
+ [Object("A"), Object("B"), Object("D")]
+ >>> [grid[1, j] for j in xrange(grid.width)]
+ [None, Object("C"), None]
+
+ Now, we can group the objects `A` and `D` to have them near one
+ another:
+
+ >>> grid = DiagramGrid(diagram, groups=FiniteSet(FiniteSet(A, D), B, C))
@ness01
ness01 Jul 22, 2012

I think the "groups" argument should accept arbitrary iterables (i.e. tuples, lists, sets, FiniteSets, whatever).

@ness01
ness01 Aug 5, 2012

Change this docstring to not make it look like you have to pass FiniteSey, just pass a list or tuple.

@scolobb

Ok. I have been sequentially through all the commits. Almost everything is fine, except:

handling of cases where the algorithm does not succeed

Fixed.

DiagramGrid has absolutely no pretty printing

I will restate it here that DiagramGrid does not derive from Basic, so I'm not sure whether I can benefit from the standard printers. Therefore, do you think I should something like a grid (or objects) property to DiagramGrid, which will return the list of lists?

The point is that a DiagramGrid is really not meant to be printed. In fact, if one wants to see what it is, one is advised to make use of the XypicDiagramDrawer class from the other pull request :-)

some nitpicking

Fixed.

Let me know when these are fixed, and I will look through the stuff again.

I think I have visited and addressed all of your comments, save for pretty printing. I have therefore rebased this branch on top of the new master and pushed it here. I have also rebased the other pull request on this one (as this is how they are meant to be) and pushed it to GitHub as well.

@scolobb

As you can see, I have added a dozen new commits. The first commit among these is fc39fd1, "DiagramGrid: Comment _triangle_objects and _other_vertex.". I did make some minor changes to the previous commits as well, but they are small in number and mostly insignificant, so I guess we will be fine if you only read through the new commits.

@travisbot

This pull request fails (merged a3471f39 into c84e5df).

@scolobb

I believe these failures are related to the FiniteSet sort key issue, which I am going to address in a different pull request, then rebase this one upon that fix and modify the tests accordingly.

@Krastanov
SymPy member

SymPy Bot Summary: 🔴 There were test failures.

@scolobb: Please fix the test failures.

Test command: setup.py test
master hash: c84e5df
branch hash: a3471f396d35defc21e6edea7a973676ad13f375

Interpreter 1: 🔴 There were test failures.

Interpreter: /usr/local/bin/python2.5 (2.5.6-final-0)
Architecture: Linux (64-bit)
Cache: yes

Test results html report: http://reviews.sympy.org/report/agZzeW1weTNyDAsSBFRhc2sY7fIgDA

Interpreter 2: 🔴 There were test failures.

Interpreter: /usr/bin/python2.7 (2.7.3-candidate-2)
Architecture: Linux (64-bit)
Cache: yes

Test results html report: http://reviews.sympy.org/report/agZzeW1weTNyDAsSBFRhc2sY3KEhDA

Interpreter 3: 🔴 There were test failures.

Interpreter: /usr/bin/python3.2 (3.2.3-candidate-2)
Architecture: Linux (64-bit)
Cache: yes

Test results html report: http://reviews.sympy.org/report/agZzeW1weTNyDAsSBFRhc2sYmrkhDA

Build HTML Docs: 🔴 There were test failures.

Docs build command: make html-errors
Sphinx version: 1.1.3

Test results html report: http://reviews.sympy.org/report/agZzeW1weTNyDAsSBFRhc2sYsNghDA

Automatic review by SymPy Bot.

@ness01
@travisbot

This pull request fails (merged 72adcfab into c84e5df).

@Krastanov
SymPy member

SymPy Bot Summary: 🔴 There were test failures.

@scolobb: Please fix the test failures.

Test command: setup.py test
master hash: 82b045f
branch hash: 72adcfab33529b6c80e89ffc2eebf3a720c17e59

Interpreter 1: 🔴 There were test failures.

Interpreter: /usr/local/bin/python2.5 (2.5.6-final-0)
Architecture: Linux (64-bit)
Cache: yes

Test results html report: http://reviews.sympy.org/report/agZzeW1weTNyDAsSBFRhc2sYuP8hDA

Interpreter 2: 🔴 There were test failures.

Interpreter: /usr/bin/python2.7 (2.7.3-candidate-2)
Architecture: Linux (64-bit)
Cache: yes

Test results html report: http://reviews.sympy.org/report/agZzeW1weTNyDAsSBFRhc2sY75YiDA

Interpreter 3: 🔴 There were test failures.

Interpreter: /usr/bin/python3.2 (3.2.3-candidate-2)
Architecture: Linux (64-bit)
Cache: yes

Test results html report: http://reviews.sympy.org/report/agZzeW1weTNyDAsSBFRhc2sYho8iDA

Build HTML Docs: 🔴 There were test failures.

Docs build command: make html-errors
Sphinx version: 1.1.3

Test results html report: http://reviews.sympy.org/report/agZzeW1weTNyDAsSBFRhc2sYpa4iDA

Automatic review by SymPy Bot.

@scolobb

This branch is now based on #1446. This has created a bit of a mess, sorry for that :-( I was too eager to finally see all the tests pass.

Let's have #1446 merged, and I will rebase this branch again, thereby removing those extraneous commits which now show at the beginning of the history.

@travisbot

This pull request fails (merged d1d0274a into 9ad73da).

@travisbot

This pull request passes (merged 72e47f93 into 9ad73da).

@Krastanov
SymPy member

SymPy Bot Summary: 🔴 There were test failures.

@scolobb: Please fix the test failures.

Test command: setup.py test
master hash: 5e2cf47
branch hash: 72e47f93e861ba638c673e5491ed04421ded006a

Interpreter 1: ✳️ All tests have passed.

Interpreter: /usr/local/bin/python2.5 (2.5.6-final-0)
Architecture: Linux (64-bit)
Cache: yes

Test results html report: http://reviews.sympy.org/report/agZzeW1weTNyDAsSBFRhc2sYlbYiDA

Interpreter 2: ✳️ All tests have passed.

Interpreter: /usr/bin/python2.7 (2.7.3-candidate-2)
Architecture: Linux (64-bit)
Cache: yes

Test results html report: http://reviews.sympy.org/report/agZzeW1weTNyDAsSBFRhc2sYg-UiDA

Interpreter 3: 🔴 There were test failures.

Interpreter: /usr/bin/python3.2 (3.2.3-candidate-2)
Architecture: Linux (64-bit)
Cache: yes

Test results html report: http://reviews.sympy.org/report/agZzeW1weTNyDAsSBFRhc2sYr64iDA

Build HTML Docs: ✳️ All tests have passed.

Docs build command: make html-errors
Sphinx version: 1.1.3

Test results html report: http://reviews.sympy.org/report/agZzeW1weTNyDAsSBFRhc2sY1vQiDA

Automatic review by SymPy Bot.

@travisbot

This pull request fails (merged 16434d1b into 9ad73da).

@ness01

This needs rebasing.

@scolobb

This needs rebasing.

Sure, done.

@travisbot

This pull request fails (merged 860bd2e5 into 7d92368).

@scolobb

The fix (hopefully) is in #1464. Sorry for the additional fuss.

scolobb added some commits Jun 17, 2012
@scolobb scolobb Start the diagram_drawing module.
This commit adds the new module, a docstring, and the declaration of
DiagramOutline.  The class itself will be implemented later.
f679274
@scolobb scolobb Add _GrowableGrid and corresponding tests.
This class represents a simple grid (two-dimensional array) which can be
grown in any of the four directions by appending empty rows and columns.
c999b2d
@scolobb scolobb Have Diagram add components of composite morphisms automatically.
Previously, diagram would not add the components of composite morphisms.
This leaved some subtle ambiguities as to how to handle composite
morphisms which had some components not included into the diagram.  This
commit fixes those ambiguities.
baf28a4
@scolobb scolobb Add some rudimentary tests for DiagramGrid.
The goal of having this tests is assuring that DiagramGrid.__new__ is
invoked.  Further tests will come together the corresponding
functionality.
4e53dbe
@scolobb scolobb DiagramGrid: Simplify the premises and conclusions and merge them.
At the drawing stage, the separation between the premises and
conclusions of the diagram is not relevant; DiagramGrid discards this
separation.  If a morphism is included both in premises and in
conclusions, after the merging the morphism will have the properties it
had in conclusions.
285d64a
@scolobb scolobb DiagramGrid: Build the undirected skeleton.
The skeleton of a diagram is the (undirected) graph whose vertices are
the objects of the diagram and which contains an edge between any pair
of objects between which in (the simplified, cf. previous commit)
diagram there is at least one morphism.  The skeleton also includes
edges between vertices A and C, such that there exists and object B
which is connected by at least one morphism with A and at least one
morphism with C.

The construction of this skeleton is one of the core tricks of the
layouting functionality.  The skeleton is constructed in such a way that
it can always be split into triangles.

Note that the term "skeleton" as used in the code, is defined ad-hoc.
824d760
@scolobb scolobb DiagramGrid: Build meaningful triangles from the skeleton of the diag…
…ram.

DiagramGrid now sees which triangles are there in the skeleton, but also
discards the triangles which have less than 2 edges corresponding to
morphisms.  Such triangles may appear because of the how the skeleton is
constructed (cf. previous commit) and are irrelevant to laying the
diagram out, because only one edge of them will eventually be shown.
Note that such triangles are actually redundant in the sense that the
objects and morphisms they refer to will eventually be pulled into the
grid by other, more meaningful triangles.  This redundancy arises from
how the triangles are built and may be a point of further improvement
and/or optimisation.
a2c394e
@scolobb scolobb DiagramGrid: Add some triangle utility functions.
This commit adds _triangle_objects which returns the set of vertices
of a triangle and _other_vertex, which, given a triangle and an edge,
returns the vertex which opposes the edge.
90ee353
@scolobb scolobb DiagramGrid: Lay down the beginnings of the main construction loop.
This commit enables DiagramGrid to add the first edge to the layout and
introduces the (currently empty) function _weld_triangle, which welds
one triangle to the current fringe.

All triangles which are relevant to the plot are assumed to form a
prefix of the list of triangles.  Whether this is true will be later
revealed by further tests and the details of functionality.
75b080b
@scolobb scolobb DiagramGrid: Find the place to attach a triangle to the existing fringe.
This commit adds and utilises _find_triangle_welding, which finds a
place in the fringe to weld the supplied triangle to.
23ecbc7
@scolobb scolobb DiagramGrid: Prepare to fixing some of the degenerate situations in t…
…he fringe.

This commit adds _fix_degenerate which, given an edge from the fringe
and two points, checks whether one of the two points defines an edge,
which is in the fringe, and which is perpendicular on the given edge.
If such a situation is found, _fix_degenerate drops the two edges and
adds the corresponding diagonal to the fringe.
c287055
@scolobb scolobb DiagramGrid: Decide how to place the outstanding vertex of the welded…
… triangle.

While _find_triangle_welding shows how to place one edge of the
triangle, it is important and nontrivial to decide where to place the
third vertex.  DiagramGrid is now capable to make this decision, while
also fixing some possible degenerate situations in the fringe.
cbde034
@scolobb scolobb DiagramGrid: Finalise the basic layouting functionality.
DiagramGrid.__new__ now can cope with some simple diagrams.  It can
maybe cope with more complex diagrams; I don't know yet.
4cccb9a
@scolobb scolobb DiagramGrid: Add _choose_target_cell.
This commit factors out the functionality of choosing one of the two
points above or below of a horizontal edge and to the left or to the
right of a vertical edge.  The new function will be later extended to
allow Diagram to avoid some of the intersections of the representations
of morphisms.
bd23826
@scolobb scolobb DiagramGrid: Extend _choose_target_cell to choose the point better.
This commit enables _choose_target_cell (cf. previous commit) to
intelligently treat the situation when both points are free.  If this is
the case, the function now makes such a choice that, should the triangle
have only two visible edges, the edge different from the welding edge
will be drawn perpendicular to the welding edge.  This will eventually
help avoid some of the intersections of morphisms.
bfbda05
@scolobb scolobb DiagramGrid: Use _put_object also to update the target cell.
If _put_object prepends the grid with a row or a column, the coordinates
of the target cell become different as well.  This must be taken into
account.
41703a2
@scolobb scolobb DiagramGrid: Remove _fix_degenerate.
It turns out that the situations which I considered when implementing
_fix_degenerate are not at all degenerate.  Therefore, at the moment,
very little is done to minimise the size of the fringe after adding a
triangle to the grid.  I will analyse the question of keeping only the
outer edges of the structure in the fringe later.
ccbb32e
@scolobb scolobb DiagramGrid: Keep track of the objects added to the diagram.
DiagramGrid now stops laying out the diagram when all objects have been
added to the grid.  Previously, it would zealously work on until all
triangles have been exhausted.
a74980a
@scolobb scolobb DiagramGrid: Prefer going down and to the right when building diagrams.
This will in most cases result in a more natural placing of the objects
of the diagram, because of how humans read from left to right, from top
to bottom.
92b1fce
@scolobb scolobb DiagramGrid: Fix the heuristics for welding triangles to vertical edges.
Before this commit DiagramGrid would wrongly choose where to put the
third vertex of the welded triangle, which would result in unnecessary
diagonal edges.
cb2c081
@scolobb scolobb DiagramGrid: Assure independence of hash randomisation.
Previously DiagramGrid would often layout the same diagrams differently
because the list of triangles was sorted differently, depending on their
(randomised) hash.
0f1af7c
@scolobb scolobb Don't derive DiagramGrid from Basic.
While DiagramGrid is essentially immutable, it does not model a
mathematical object and therefore should not be a subclass of Basic.
e509078
@scolobb scolobb DiagramGrid: Store the grid of objects in the instance and offer acce…
…ss to it.

This commit makes it possible to use DiagramGrid from the outer world:
the class now stores the results of the analysis it has performed.
b67baa0
@scolobb scolobb Add some proper tests for DiagramGrid.
Since DiagramGrid is already usable now, it is now possible to test how
some simple diagrams are laid out.
5fa96dd
@scolobb scolobb DiagramGrid: Don't re-place objects on the grid.
After each new triangle placed in the grid, forget all triangles which
only include objects which have already been placed on the grid.
24a675c
@scolobb scolobb DiagramGrid: Add a test for a strange diagram.
This test attempts to use DiagramGrid to lay out a diagram which
resulted from a typo in an attempt to lay out the  the five lemma.
DiagramGrid currently fails to lay it out properly because of
the fact that en route it encounters a situation where none of the
remaining triangles can be welded to the already constructed structure.
515ae01
@scolobb scolobb DiagramGrid: Attach triangles by vertices when this cannot be done by…
… edges.

This commit enables DiagramGrid to lay out more complex diagrams, like
the five lemma.  Generally, using the function _grow_pseudopod, added in
this commit, should be avoided for as long as possible, and this is what
DiagramGrid currently does.
9761974
@scolobb scolobb DiagramGrid: Test how DiagramGrid lays out a cube diagram.
The new test requires DiagramGrid to lay out a cube.  Rather obviously,
the result is not a cube, because of how DiagramGrid is designed.
42ccc71
@scolobb scolobb DiagramGrid: Test a pullback.
This test currently fails because the loop in the constructor of
DiagramGrid fails to distinguish between the situations when
_weld_triangle has not found a welding and when it has corrected the
fringe and thus requires a restart of the search for weldings.
2be0f5c
@scolobb scolobb DiagramGrid: Distinguish between welding failures and restarts.
The loop in the constructor DiagramGrid now can distinguish between the
situations when _weld_triangle has not found any welding and when it has
corrected the fringe and requires a restart of the process.  This makes
the test added in the previous commit pass.
8d3e193
@scolobb scolobb DiagramGrid: Lay out one-morphism diagrams as well.
This commit also adds a corresponding test.
86d325b
@scolobb scolobb Diagram: Add is_subdiagram and test it.
This method checks whether its argument is a subdiagram of self.
49f2ed3
@scolobb scolobb Diagram: Fix the mess in handling conclusions.
Diagram would previously add the components of composite morphisms which
belonged to conclusions, which would clutter the conclusions.
Furthermore, it wouldn't sometimes properly add properties to the
morphisms in conclusions.  This commit fixes all of these.
4d88b4b
@scolobb scolobb Diagram: Add subdiagram_from_objects.
This method returns a subdiagram of self which includes the objects
supplied in the argument.
ee8bbdc
@scolobb scolobb DiagramGrid: Add the possibility to lay out diagrams in logical groups.
The constructor of DiagramGrid now accepts the argument groups, which
specifies groups of objects to be laid out independently from the others
in their own group.  This commit also adds the corresponding tests.
1fd263f
@scolobb scolobb DiagramGrid: Test the five lemma.
I have just discovered that what I initially thought to be tests for the
five lemma has turned to be a weird diagram resulted from a typo.  This
commit adds the test for five lemma proper.
d246cbf
@scolobb scolobb DiagramGrid: Test the five lemma with grouping of objects.
Grouping objects allows laying diagrams out in better structures.  For
example, with grouping, it is possible to make the automated layout of
the five lemma look more like how it is usually drawn by hand.
731e099
@scolobb scolobb Make _GrowableGrid and DiagramGrid derive from object.
This transforms these classes into new-style classes.
fab82b6
@scolobb scolobb DiagramGrid: Factor out the generic laying out functionality.
In the future, the constructor of DiagramGrid will live through further
enhancements, like hints, which makes it important to push what it does
to a higher level of abstraction.
43a9dc5
@scolobb scolobb DiagramGrid: Implement the sequential layout for diagrams.
This layout method attempts to place the objects as closed to a line as
possible.  This commit also adds the corresponding tests.
b173213
@scolobb scolobb DiagramGrid.__init__: Rearrange the code in a big if statement.
This commit makes the code in the constructor of DiagramGrid more
orderly and easy to extend later.
1dfd0e4
@scolobb scolobb DiagramGrid: Implement the transpose hint.
If transpose == True, the constructor of DiagramGrid will transpose the
resulting grid.
236425c
@scolobb scolobb DiagramGrid: Test the layout of a pullback with sequential layout.
This test is there just for stress testing the sequential layout,
because the pullback doesn't really look like nice when laid out with
this strategy.
24f0f5a
@scolobb scolobb DiagramGrid: Rename hint "shape" to "layout". 6bd40c0
@scolobb scolobb DiagramGrid: Rearrange the tests a bit.
This commit assures a slightly better logical grouping of the tests.
ff5223f
@scolobb scolobb DiagramGrid: Support hints in groups as well.
Using only the two supported hints it is not possible to lay out the
five lemma almost exactly as it is normally laid out.  This commit adds
a test to show off this new capability.
84f50b2
@scolobb scolobb DiagramGrid: Test a pull back laid out with generic layout.
Somewhere in the past I have accidentally removed the tests for a pull
back laid out with generic layout.  This test brings that code back.
38e0681
@scolobb scolobb DiagramGrid._triangle_key: Remove the necessity of objects to have na…
…mes.

This method used to produced a key for sorting the triangles which was
dependent on objects having names.  This would fail for groups, where
objects are FiniteSet's.

Note that this method currently sorts the set of objects in order to
avoid the problem of the sort key of FiniteSet being different in
dependence of the order of the elements in it.
b421413
@scolobb scolobb DiagramGrid._handle_groups: Fix a bug in copying child grids. 2b700da
@scolobb scolobb DiagramGrid: Write an extensive docstring.
This docstring includes explanations and examples for all features
available to the user.
20f9625
@scolobb scolobb DiagramGrid: Add examples for width, height, and __getitem__. ce5ec82
@scolobb scolobb docs/modules/categories.rst: Add a reference to DiagramGrid. fec3bc0
@scolobb scolobb diagram_drawing: Describe the layout algorithm in the module docstring. 0cae466
@scolobb scolobb DiagramGrid: Store the simplified and merged morphisms.
This commit adds the property morphisms which returns the dictionary
mapping those morphisms to their properties which were considered worthy
by the constructor of DiagramGrid.
1c1e149
@scolobb scolobb DiagramGrid: Sort the vertices in the root triangle.
Previously DiagramGrid would not sort the vertices in the edges of
the first triangle, out of which the root edge was picked.  This resulted
in different edges being sometimes picked, as well arbitrary positioning
of the vertices of the root edge.
cbfdf55
@scolobb scolobb DiagramGrid: Don't use "loop" morphisms in laying out objects.
"Loop" morphisms are morphisms which have the same domains and
codomains.  They will be eventually drawn as loops and should not
influence the layout of the diagram.
de5a5b2
@scolobb scolobb DiagramGrid: Comment _triangle_objects and _other_vertex.
The code of these two functions might be a little bit difficult to
comprehend; this commits adds what I believe to be extensive comments
explaining how and why these methods work.
8c7ae4f
@scolobb scolobb Rename Diagram._add_morphism -> Diagram._add_morphism_closure.
The new name describes what the method does a little bit better, and
also removes the possible confusion with
CompositeMorphism._add_morphism.
c738ab8
@scolobb scolobb DiagramGrid._merge_premises_conclusions: Use itertools.chain.
This simplifies the method and outsources the functionality to better
functions.
4926c1e
@scolobb scolobb Fix some minor typos and space issues in DiagramGrid-related code. c679614
@scolobb scolobb DiagramGrid: Use set's and frozenset's instead of FiniteSet.
This commit changes the code which needs sets for internal operations to
use the builtins set and frozenset instead of FiniteSet.
64d6242
@scolobb scolobb DiagramGrid: Fix some redundant line breaks.
In a number of places (especially in list comprehensions) I used the
backslash to assure proper interpretation; it turns out a number of such
backslashes were not necessary.
3a0342b
@scolobb scolobb _compute_triangle_min_sizes: Explain why this metric.
This commit adds a high-level explanation of the heuristic triangle
metric implemented in this function.
d7bb7e4
@scolobb scolobb DiagramGrid._put_object: Make ``offset`` values clearer. 3e3d55e
@scolobb scolobb DiagramGrid: Refactor _weld_triangle.
This method used to melange the missions of finding the triangle to weld
and actually welding the triangle.  This resulted in the necessity to
return a triangle or one of the two status codes.  This commit fixes
this ugliness by better splitting the functionality between __init__,
_weld_triangle and _find_triangle_welding (which is not called
_find_triangle_to_weld).
df16cfd
@scolobb scolobb DiagramGrid: Store edges in two-element frozensets.
This commit transitions from storing edges as two-element tuples to
storing them as frozensets.  The rationale behind this change is that
the edges of the skeleton are undirected, which makes storing them in
tuples inconvenient and results in a lot of uglier code.
9977109
@scolobb scolobb DiagramGrid: Add _get_undirected_graph.
This commit factors out of _sequential_layout the code producing the
adjacency lists of the underlying undirected graph of the diagram.
528318f
@scolobb scolobb DiagramGrid: Add support for disconnected diagrams.
DiagramGrid is now capable of laying out disconnected diagrams by laying
out each connected component separately.  The grids of each of the
connected components are arranged in a line.  This is a place where
further improvements could be introduced.
ddfd51f
@scolobb scolobb DiagramGrid: Support one-object diagrams.
While one doesn't construct one-object diagrams every day, this is a
valid edge case that has to be supported.  Furthermore, laying out
one-object diagrams will be useful for some internal purposes (see the
next commits).
5a3d764
@scolobb scolobb DiagramGrid: Handle the situations when growing a pseudopod fails.
While I initially supposed that reaching such situations would be
difficult, it proved quite easy to get there by setting up a "star"
diagram, with many morphisms coming out of a central object.  This
commit adds the code to handle such situations and actually lay out all
objects.
7821c42
@scolobb scolobb Diagram: Disallow identity morphisms to have properties.
Identity morphisms are trivial and properties for them do not make
sense.  This should not be confused with nontrivial morphisms with
the same domain and codomain, which are instances of NamedMorphism and
can always be annotated with properties.
ef2dc14
@scolobb scolobb DiagramGrid: Use implicit generator expressions where possible.
This commit fixes the expressions func([<generator-expression>]) to look
like func(<generator-expression>), i.e., removes the redundant square
brackets.
dcfc319
@scolobb scolobb DiagramGrid: Check out the transpose hint nicer.
Previously, the code would check out the value of the transpose hint
using two if statements.  Now it does so in one if statement.
0686a9e
@scolobb scolobb DiagramGrid._handle_groups: Add lay_out_group.
Previously, the part of _handle_groups that laid out each of the group
would consist of two largely similar halves: one that processed groups
supplied as an array of FiniteSet's, and the other one which processed
groups as a dictionary mapping groups to their hints.  This commit
factors out the common part into lay_out_group.
efb8858
@scolobb scolobb DiagramGrid: Support any iterable to describe groups.
Previously DiagramGrid would only agree if groups were supplied as
FiniteSet's of FiniteSet's.  This commit enables DiagramGrid to cope
with any iterable type.  The test added hereby shows how this can be
utilised.
0d69ed4
@scolobb scolobb DiagramGrid: Add LaTeX pretty printing and __str__.
This makes DiagramGrid easier to show in doctests and, probably, easier
to use.  Pretty printing to string will come in the next commits.
8292883
@scolobb scolobb DiagramGrid: Upgrade the docstrings to use string printing.
Previously, the docstrings would print the grid object by object.  Now
it relies on DiagramGrid.__str__ to show the contents of the grid.
93533c7
@scolobb scolobb Factor _print_matrix_contents out of _print_MatrixBase.
This commit adds to the string pretty printer the method
_print_matrix_contents which only carries that part of the previous
version of _print_MatrixBase which refers to printing the grid.
_print_MatrixBase now only puts this grid in brackets.

The motivation behind this commit is that grid printing is useful in
itself and thus should be available as a separate method.  See the next
commit for an example of such usage.
b7e5772
@scolobb scolobb DiagramGrid: Add string pretty printing.
DiagramGrid is not printing use the core functionality of matrix
printing.  This code should serve as an example that printing stuff in a
grid is useful not only in matrix printing.
33bef6e
@scolobb scolobb Fix DiagramGrid tests to work with hash-stabilised FiniteSet and Dict.
This commit fixes the tests for DiagramGrid to test the correct,
hash-independent outcome.  Previously, the tests would test for one of
the possible outcomes.
1f241da
@scolobb scolobb DiagramGrid._grow_pseudopod: Remove a redundant check.
This commit removes a check for an impossible situation.  The
explanation is provided in a comment.
387229e
@scolobb scolobb DiagramGrid._triangle_key: Remove the TODO notice.
This TODO notice was valid when triangles were stored as FiniteSet's.
Now that triangles are stored as frozenset's, it is actually necessary
to sort the objects explicitly, because frozenset's do not have sort
keys.
a8b6acd
@scolobb scolobb DiagramGrid._grow_pseudopod: Fix a very ugly mistake.
Initially I forgot to pick a certain element from the list of triangles
which could be attached and used the implicit value of the loop
variable, which would be the last element of the list.  This commit
fixes this mistake by explicitly picking the first triangle in the list.

This change has slightly changed the result for a test case; this commit
fixes the test as well.
ccc3b76
@scolobb scolobb DiagramGrid._grow_pseudopod: Properly sort the edges.
The previous version of the code essentially invoked default_sort_key on
a frozenset, which didn't obviously produce the necessary result.
14489d5
@travisbot

This pull request passes (merged ccc3b76 into 41e968b).

@ness01

This is ready to go in, isn't it?

@ness01 ness01 commented on an outdated diff Aug 5, 2012
sympy/categories/baseclasses.py
@@ -814,3 +840,77 @@ def hom(self, A, B):
conclusions |= FiniteSet(morphism)
return (premises, conclusions)
+
+ def is_subdiagram(self, diagram):
+ """
+ Checks whether ``diagram`` is a subdiagram of ``self``.
+ Diagram `D'` is a subdiagram of `D` if all premises
+ (conclusions) of `D'` are contained in the premises
+ (conclusions) of `D`. The morphisms contained
+ both in `D'` and `D` should have the same properties for `D'`
+ to be a subdiagram of `D`.
+
+ Examples
+ ========
+ >>> from sympy.categories import Object, NamedMorphism, Diagram
+ >>> from sympy import FiniteSet, pretty
@ness01
ness01 Aug 5, 2012

Why this line?

@ness01 ness01 commented on an outdated diff Aug 5, 2012
sympy/categories/baseclasses.py
+ """
+ If ``objects`` is a subset of the objects of ``self``, returns
+ a diagram which has as premises all those premises of ``self``
+ which have a domains and codomains in ``objects``, likewise
+ for conclusions. Properties are preserved.
+
+ Examples
+ ========
+ >>> from sympy.categories import Object, NamedMorphism, Diagram
+ >>> from sympy import FiniteSet, pretty
+ >>> A = Object("A")
+ >>> B = Object("B")
+ >>> C = Object("C")
+ >>> f = NamedMorphism(A, B, "f")
+ >>> g = NamedMorphism(B, C, "g")
+ >>> d = Diagram([f, g], {f:"unique", g*f:"veryunique"})
@ness01
ness01 Aug 5, 2012

missing space after colon

@ness01 ness01 commented on an outdated diff Aug 5, 2012
sympy/categories/baseclasses.py
+ for m in diagram.conclusions])
+
+ # Premises is surely ``True`` here.
+ return conclusions
+
+ def subdiagram_from_objects(self, objects):
+ """
+ If ``objects`` is a subset of the objects of ``self``, returns
+ a diagram which has as premises all those premises of ``self``
+ which have a domains and codomains in ``objects``, likewise
+ for conclusions. Properties are preserved.
+
+ Examples
+ ========
+ >>> from sympy.categories import Object, NamedMorphism, Diagram
+ >>> from sympy import FiniteSet, pretty
@ness01
ness01 Aug 5, 2012

We don't need pretty, do we?

@ness01 ness01 and 1 other commented on an outdated diff Aug 5, 2012
sympy/categories/diagram_drawing.py
+ >>> from sympy.categories import Object, NamedMorphism
+ >>> from sympy.categories import Diagram, DiagramGrid
+ >>> from sympy import FiniteSet
+ >>> A = Object("A")
+ >>> B = Object("B")
+ >>> C = Object("C")
+ >>> f = NamedMorphism(A, B, "f")
+ >>> g = NamedMorphism(B, C, "g")
+ >>> diagram = Diagram([f, g])
+
+ The simplest way to have a diagram laid out is the following:
+
+ >>> grid = DiagramGrid(diagram)
+ >>> (grid.width, grid.height)
+ (2, 2)
+ >>> print grid
@ness01
ness01 Aug 5, 2012

how about pretty(grid)?

@scolobb
scolobb Aug 7, 2012

That's a very cool idea, thanks for spotting it!

@ness01 ness01 commented on an outdated diff Aug 5, 2012
sympy/categories/diagram_drawing.py
+ for i in xrange(self._grid.height):
+ for j in xrange(self._grid.width):
+ grid[j, i] = self._grid[i, j]
+ self._grid = grid
+
+ @property
+ def width(self):
+ """
+ Returns the number of columns in this diagram layout.
+
+ Examples
+ ========
+
+ >>> from sympy.categories import Object, NamedMorphism
+ >>> from sympy.categories import Diagram, DiagramGrid
+ >>> from sympy import FiniteSet
@ness01
ness01 Aug 5, 2012

No need for this line.

@ness01 ness01 commented on an outdated diff Aug 5, 2012
sympy/categories/diagram_drawing.py
+ 2
+
+ """
+ return self._grid.width
+
+ @property
+ def height(self):
+ """
+ Returns the number of rows in this diagram layout.
+
+ Examples
+ ========
+
+ >>> from sympy.categories import Object, NamedMorphism
+ >>> from sympy.categories import Diagram, DiagramGrid
+ >>> from sympy import FiniteSet
@ness01 ness01 commented on an outdated diff Aug 5, 2012
sympy/categories/tests/test_baseclasses.py
@@ -124,7 +124,7 @@ def test_diagram():
# Make sure that (re-)adding composites (with new properties)
# works as expected.
d = Diagram([f, g], {g * f:"unique"})
- assert d.conclusions[g * f] == FiniteSet("unique")
+ assert d.conclusions == Dict({g*f:FiniteSet("unique")})
@ness01
ness01 Aug 5, 2012

space after colon

@ness01

Modulo my nitpicks, I think this is ready to go in.

As a side note, does this work in python3.3? I tried to check, but I get weird errors all over the place (not related to categories, as far as I can tell):

================================================================================================================================ test process starts =================================================================================================================================
executable:         /usr/bin/python3.3  (3.3.0-beta-1)
architecture:       64-bit
cache:              yes
ground types:       python
random seed:        7848159
hash randomization: on (PYTHONHASHSEED=1798822454)

sympy/categories/tests/test_baseclasses.py[3] .E.                                                                                                                                                                                                                               [FAIL]
sympy/categories/tests/test_drawing.py[2] .E                                                                                                                                                                                                                                    [FAIL]

______________________________________________________________________________________________________________________________________________________________________________________________________________________________________________________________________________________
______________________________________________________________________________________________________________ sympy/categories/tests/test_baseclasses.py:test_diagram _______________________________________________________________________________________________________________
  File "/home/ness/src/sympy/py3k-sympy/sympy/categories/tests/test_baseclasses.py", line 121, in test_diagram
    d11 = Diagram({f:"unique"})
  File "./sympy/categories/baseclasses.py", line 700, in __new__
    premises, morphism, FiniteSet(props))
  File "./sympy/categories/baseclasses.py", line 618, in _add_morphism_closure
    new_props = existing_props & props
  File "./sympy/core/sets.py", line 237, in __and__
    return self.intersect(other)
  File "./sympy/core/sets.py", line 84, in intersect
    return Intersection(self, other)
  File "./sympy/core/sets.py", line 927, in __new__
    return Intersection.reduce(args)
  File "./sympy/core/sets.py", line 981, in reduce
    return s.__class__(x for x in s
  File "./sympy/core/sets.py", line 1144, in __new__
    args = list(map(sympify, args))
  File "./sympy/core/sets.py", line 982, in <genexpr>
    if all(x in other for other in args))
  File "./sympy/core/sets.py", line 982, in <genexpr>
    if all(x in other for other in args))
  File "./sympy/core/sets.py", line 258, in __contains__
    result = ask(symb)
  File "./sympy/assumptions/ask.py", line 86, in ask
    res = key(expr)._eval_ask(assumptions)
  File "./sympy/assumptions/assume.py", line 88, in _eval_ask
    return self.func.eval(self.arg, assumptions)
  File "./sympy/assumptions/assume.py", line 147, in eval
    cls = get_class(handler)
  File "./sympy/utilities/source.py", line 27, in get_class
    lookup_view = getattr(__import__(mod_name, {}, {}, ['']), func_name)
ImportError: No module named 'sympy.assumptions.handlers.'
______________________________________________________________________________________________________________________________________________________________________________________________________________________________________________________________________________________
______________________________________________________________________________________________________________ sympy/categories/tests/test_drawing.py:test_DiagramGrid _______________________________________________________________________________________________________________
  File "/home/ness/src/sympy/py3k-sympy/sympy/categories/tests/test_drawing.py", line 96, in test_DiagramGrid
    d = Diagram([f, g], {g * f:"unique"})
  File "./sympy/categories/baseclasses.py", line 726, in __new__
    if (morphism.domain in objects) and \
  File "./sympy/core/sets.py", line 258, in __contains__
    result = ask(symb)
  File "./sympy/assumptions/ask.py", line 86, in ask
    res = key(expr)._eval_ask(assumptions)
  File "./sympy/assumptions/assume.py", line 88, in _eval_ask
    return self.func.eval(self.arg, assumptions)
  File "./sympy/assumptions/assume.py", line 147, in eval
    cls = get_class(handler)
  File "./sympy/utilities/source.py", line 27, in get_class
    lookup_view = getattr(__import__(mod_name, {}, {}, ['']), func_name)
ImportError: No module named 'sympy.assumptions.handlers.'

============================================================================================================== tests finished: 3 passed, 2 exceptions, in 0.17 seconds ===============================================================================================================
DO *NOT* COMMIT!
@Krastanov
SymPy member

SymPy Bot Summary: ✳️ All tests have passed.

Test command: setup.py test
master hash: 1627b32
branch hash: ccc3b76

Interpreter 1: ✳️ All tests have passed.

Interpreter: /usr/local/bin/python2.5 (2.5.6-final-0)
Architecture: Linux (64-bit)
Cache: yes

Test results html report: http://reviews.sympy.org/report/agZzeW1weTNyDAsSBFRhc2sY7JsjDA

Interpreter 2: ✳️ All tests have passed.

Interpreter: /usr/bin/python2.7 (2.7.3-candidate-2)
Architecture: Linux (64-bit)
Cache: yes

Test results html report: http://reviews.sympy.org/report/agZzeW1weTNyDAsSBFRhc2sY6_8hDA

Interpreter 3: ✳️ All tests have passed.

Interpreter: /usr/bin/python3.2 (3.2.3-candidate-2)
Architecture: Linux (64-bit)
Cache: yes

Test results html report: http://reviews.sympy.org/report/agZzeW1weTNyDAsSBFRhc2sY9fQiDA

Build HTML Docs: ✳️ All tests have passed.

Docs build command: make html-errors
Sphinx version: 1.1.3

Test results html report: http://reviews.sympy.org/report/agZzeW1weTNyDAsSBFRhc2sYt6sjDA

Automatic review by SymPy Bot.

@ness01

Great, so the tests pass. @scolobb this can go in whenever you're done with the nitpicks.

scolobb added some commits Aug 7, 2012
@scolobb scolobb Brush the doctests a little bit.
This commit removes bogus imports, as well as fixes an example to not
use ``FiniteSet`` to supply groups to ``DiagramGrid``.
3e23f68
@scolobb scolobb Comply with PEP8 whitespace recommendations in writing dicts.
This commit fixes the whitespace after the colon in dicts.  Previously,
I would sometimes not write them.
2704fc5
@scolobb scolobb DiagramGrid: Use pretty printing in the main docstring. 47b7640
@travisbot

This pull request passes (merged 47b7640 into 41e968b).

@scolobb

@ness01 , thanks for the extra round of review! I have fixed everything you have pointed out, including fixing all whitespace in writing dicts.

I get the same errors with Python 3.3, but I get them all over SymPy anyway, so I believe they are hardly introduced by my code.

@ness01 ness01 merged commit 3c2c3c7 into sympy:master Aug 7, 2012

1 check passed

Details default The Travis build passed
@ness01

@scolobb solid work! I'll review diagram-drawing as soon as I can.

@scolobb

Oh yeah! Thank you very much :-)

I'm looking forward to your feedback, but be sure to only review the pull request when you really have the time.

@Krastanov
SymPy member

SymPy Bot Summary: ✳️ All tests have passed.

Test command: setup.py test
master hash: 3c2c3c7
branch hash: 47b7640

Interpreter 1: ✳️ All tests have passed.

Interpreter: /usr/local/bin/python2.5 (2.5.6-final-0)
Architecture: Linux (64-bit)
Cache: yes

Test results html report: http://reviews.sympy.org/report/agZzeW1weTNyDAsSBFRhc2sYl_AhDA

Interpreter 2: ✳️ All tests have passed.

Interpreter: /usr/bin/python2.7 (2.7.3-candidate-2)
Architecture: Linux (64-bit)
Cache: yes

Test results html report: http://reviews.sympy.org/report/agZzeW1weTNyDAsSBFRhc2sYvo8iDA

Interpreter 3: ✳️ All tests have passed.

Interpreter: /usr/bin/python3.2 (3.2.3-candidate-2)
Architecture: Linux (64-bit)
Cache: yes

Test results html report: http://reviews.sympy.org/report/agZzeW1weTNyDAsSBFRhc2sYj7MjDA

Build HTML Docs: ✳️ All tests have passed.

Docs build command: make html-errors
Sphinx version: 1.1.3

Test results html report: http://reviews.sympy.org/report/agZzeW1weTNyDAsSBFRhc2sYipQjDA

Automatic review by SymPy Bot.

@asmeurer
SymPy member

SymPy Bot Summary: 🔴 There were test failures (merged scolobb/ct1-diagram-layout (47b7640) into master (c7a4a4a)).

@scolobb: Please fix the test failures.

Build HTML Docs: 🔴 There were test failures.
Sphinx version: 1.1.3

@asmeurer
SymPy member

@scolobb I think these doc failures are due to this branch.

@certik
SymPy member

But otherwise great job on the patch.

@scolobb

Hm, note that Stefan's bot reported no problems, and neither do I see any errors on my box. I'll try to see what's different in Aaron's configuration.

@asmeurer
SymPy member

I also don't see anything in master. Perhaps this is yet another bug in my sympy-bot branch. Let me figure it out.

@asmeurer
SymPy member

Well, now it's passing for me even in SymPy bot. So maybe it was just fixed (?). Anyway, if it doesn't show up again, I'm not going to worry about it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment