Add order objects - #474
Conversation
6873884 to
db10e1d
Compare
Joseph-Edwards
left a comment
There was a problem hiding this comment.
A number of tweaks (mostly related to the doc and tests), but overall this looks really good! Thanks @james-d-mitchell
|
|
||
| """Subpackage for importing and wrapping :any:`du_narendran_rusinowitch`.""" | ||
|
|
||
| # pylint: disable=no-name-in-module |
There was a problem hiding this comment.
I don't think this should be necessary
There was a problem hiding this comment.
Let's discuss this, I'm also not sure why this is necessary but without it I get a warning when running pylint locally.
There was a problem hiding this comment.
The lack of structure in this file makes it a bit difficult to check we're testing the API fully. It would be better if this file was split up into different sections per order, and within each section, there were tests for:
- with alphabet
- without alphabet
- copy and repr (object only, not
_cmpfunctions) - rejects bad constructor arguments (object only, not
_cmpfunctions)
As far as I can tell, we aren't obviously missing anything, but I'm not confident in that.
Co-authored-by: Joe Edwards <80713360+Joseph-Edwards@users.noreply.github.com>
|
@Joseph-Edwards thanks for the review, I think that should be everything now. |
No description provided.