#16331: Game Theory: Build capacity to solve matching games in to Sage.
-------------------------------------+-------------------------------------
Reporter: vinceknight | Owner:
Type: enhancement | Status: needs_review
Priority: major | Milestone: sage-6.4
Component: game theory | Resolution:
Keywords: Game Theory, | Merged in:
Matching Games, | Reviewers:
Authors: | Work issues:
Report Upstream: N/A | Commit:
Branch: | 197c9ae815c4753b5fc3681b1d1a651667289ecc
u/vinceknight/game_theory__build_capacity_to_solve_matching_games_in_to_sage_|
Stopgaps:
Dependencies: |
-------------------------------------+-------------------------------------
Comment (by vinceknight):
Replying to [comment:67 tscrim]:
> Here's some more things that come from a more detailed look:
>
> - You've created another folder for the game theory called `gametheory`
whereas the existing folder `game_theory` should be used (also it is the
better name IMO).
That shouldn't be there: have deleted.
> - Is gambit (#16466) necessary for this ticket?
No, this has nothing to do with #16466, is it looking like it does?
> - Never have mutable attributes public (with no leading underscore `_`,
ex. `suitors` and `reviewers`). It's also good practice to not expose
internal-use attributes.
I mentioned to Karl that at times it might be nice to be able to access
these. Kind of like in lines 147 onwards of the docs: `sage: from
itertools import permutations ...`. Happy to remove if you guys don't
think that's a good idea.
> - Bipartite is one word, so change `bi_partite` to `bipartite`.
Easy fix: done.
> - `_dict_game` doesn't have a doctest.
Testing this was rather tricky as it was 'just something' that works in
`__init__` so I've moved it to `__init__` which makes it easier to doc-
test (I added another doc test there).
> - For the `__eq__` method of `_Player`, I think you're better comparing
`name`. In particular, this would guarantee hash correctness (if `x == y`,
then we must have `hash(x) == hash(y)`).
By using `.__repr__()` this allows us to test equality not only against
another player instance but also against a name of player which is the
values in the dictionaries... Changing this would require a re-write of
the dictionary that holds the preferences... This isn't necessarily
impossible but just checking if it's really necessary?
> - Remove the leading underscore of `_Player` so it gets included in the
doc (at least, I think it won't get included as is), and it's the proper
convention for classes. See the following note too.
Done, although just confirming that we want `Player` to appear in the
docs? This is only meant to be used internally really.
> - In the `all.py`, it's not good practice to import everything in a
particular module (file) as it makes it harder to import things into the
global namespace.
This shouldn't be there. Maybe it was in `gametheory`...
> - In the `__init__.py`, you don't need to `import all` (usually these
files are left blank).
Done.
> - Make `_Player` inherit from `object` since new-style classes are
recommended by python (or at least remove those parentheses). Same for
`Coop_Game`.
`Coop_Game` must have been in `gametheory`, have made `_Player` inherit
from `SageObject`.
> - Rename `Coop_Game` to `CoopGame`; the latter is the proper naming
convention for classes.
`Coop_Game` doesn't exist (anymore). (Or shouldn't if it does).
> - Actually, more generally in your code you've mixed the python
conventions: classes follow `CamelCase` and functions/attributes/variables
use `underscore_names`.
I believe to have caught these now.
> - `cooperativegames.py` (which probably should be renamed as
`cooperative_games.py` for easier reading) is lacking doctests. Actually,
is this redundant from `game_theory/cooperative_game.py`?
Yes it's redundant (have removed `gametheory`). Sorry...
> - Doc formatting:
> {{{
> -``name`` - Can be a string or a number. If left blank will
automatically
> generate an integer.
> }}}
> should be
> {{{
> - ``name`` -- a string or a number; if left blank will automatically
> generate an integer
> }}}
Done.
> Note the punctuation, spacing, and alignment. Also change `: ::` into
`::` (this was noted by Karl on another ticket I believe).
Done.
> - More on the docs, please make the short description into the
affirmative (I think this is the right word). So `Raises` to `Raise`,
`Constructs` to `Construct`, etc. It's a python convention.
I think I've caught all these.
>
> I'll take another look once all of this is done.
>
> A category-type question: Does it make sense to talk about the set of
all games on a fixed set of players? If so, is there some notion of a
morphism? I did some searching and came across [[http://www.pps.univ-
paris-diderot.fr/~mh/catgames.pdf|this article]] describing a category for
a particular class of games. So perhaps (on a future ticket) we reorganize
everything into using the category framework. This is more food for
thought.
Great question, probably not enough in my area of expertise to be able
answer straight away but will think about it.
--
Ticket URL: <http://trac.sagemath.org/ticket/16331#comment:69>
Sage <http://www.sagemath.org>
Sage: Creating a Viable Open Source Alternative to Magma, Maple, Mathematica,
and MATLAB
--
You received this message because you are subscribed to the Google Groups
"sage-trac" group.
To unsubscribe from this group and stop receiving emails from it, send an email
to [email protected].
To post to this group, send email to [email protected].
Visit this group at http://groups.google.com/group/sage-trac.
For more options, visit https://groups.google.com/d/optout.
