#19163: LatticePoset creation: Empty argument, better error reporting
-------------------------------------+-------------------------------------
Reporter: jmantysalo | Owner:
Type: enhancement | Status: needs_review
Priority: minor | Milestone: sage-7.1
Component: combinatorics | Resolution:
Keywords: | Merged in:
Authors: Jori Mäntysalo | Reviewers:
Report Upstream: N/A | Work issues:
Branch: | Commit:
u/jmantysalo/latticeposet_creation__empty_argument__better_error_reporting|
0d0a09a405986b478644873761d2dd59cad03dc6
Dependencies: | Stopgaps:
-------------------------------------+-------------------------------------
Comment (by tscrim):
Thank you for making the changes; it definitely improves the code. A few
comments:
- Move the comment before `LatticeException` as the docstring for that
class so it is visible in the doc.
- Could be bikeshedding, but I'm thinking we should call it `LatticeError`
since it is inheriting from `ValueError`.
- Make the default value for `x` and `y` in `LatticeException` to be
`None`.
- All methods need doctests, this includes those for the exceptions
- I would consider making the no top/bottom errors just raise
`ValueError`. Granted, this won't look as slick when the error gets
reported, but it will be cleaner code. (This would mean we don't need
default values for `LatticeException`.) IMO this is also okay since
`LatticeException` inherits from `ValueError`.
- The errors are not sentences, so they should start with a lowercase
letter.
- I would do:
{{{#!python
try:
P._hasse_diagram.meet_matrix()
except LatticeException as error:
error.x = P._vertex_to_element(error.x)
error.y = P._vertex_to_element(error.y)
raise
}}}
- Unrelated but I noticed it because there were changes made. I would do
this:
{{{#!diff
-if isinstance(data,FiniteLatticePoset) and len(args) == 0 and
len(options) == 0:
+if (isinstance(data, FiniteLatticePoset) and not args and not options):
}}}
as it is the fastest way to check for emptiness.
--
Ticket URL: <http://trac.sagemath.org/ticket/19163#comment:23>
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 https://groups.google.com/group/sage-trac.
For more options, visit https://groups.google.com/d/optout.
