Comments (12)
Why should our policy be different to Theano's? They seem to have plenty of contributors.
from lasagne.
Um, if I may chime in: in my experience with experienced/productive developers, the absence of testing can be much scarier (and thus be a barrier) than strictness: If you have full unit tests, full regression tests for bugfixes and rudimentary smoke tests that cover common usecases (by verifying that they run through), then you can be pretty much sure not to break anything existing by simply getting all the tests to pass in your contribution branch. If this is guaranteed, then it gives a feeling of safety that is worth quite a lot IMO.
By its very nature, this package should always have very easy to unit test building blocks, so it shouldn't really be that much effort to do it properly. Often one can also compare to a numpy/scipy version of the same functionality.
Also, as a more and more regular user (and contributor if I find something that I can be useful for) of this toolbox, I like the feeling to be able to rely on it working, and testably so ...
In any case, I'd be very very very wary of potential code rot and the broken window effect: If you decide now to be relaxed on testing, it will be very difficult to change policies later on.
It may also be a point of distinction to other python dl packages that are cropping up here and there, to be able to say yes, we do all that, but we also have tests.
For new contributors this may also be a great opportunity to learn good practice ...
Following @dnouri s comment: There doesn't seem to be a problem with contributor numbers in sklearn either.
from lasagne.
All good points. Then I guess we need some kind of code sprint to get the current code base up to speed with tests. We can't expect contributors to provide tests if we don't do it ourselves :) And I guess the same thing goes for documentation, actually.
from lasagne.
Cool. Yes, let's try to improve that test and docs coverage. I've been falling behind a little bit lately. But I think in the meantime we can ask contributors to add tests for new stuff where tests for that class already exist, and update docs where they're already written.
from lasagne.
we can ask contributors to [...] update docs where they're already written.
Yes, I totally agree with that. I cannot stand outdated docstrings, and I feel bad when modifying undocumented code. I will try to develop the same strong feelings for tests.
About the "this might put up a barrier for new contributors" argument: We don't have to force contributors to add all the tests and docstrings that come to our mind when reading a PR, we can ask them nicely, and do them ourselves if they're not able to comply and the contribution is worth it. There's a nice article about that: http://blog.spreedly.com/2014/06/24/merge-pull-request-considered-harmful (the first section). I guess that could be a good compromise between being overly strict with contributors and inviting the broken window effect.
from lasagne.
Very good read! I agree that that's a good compromise.
from lasagne.
To chime in with my experience from maintaining Luigi: I think in fact it actually lowers the barrier for contribution since the tests provide a safety guarantee. A ratio of 1:1 in terms of line count is generally a good goal.
from lasagne.
Regarding the 1:1 ratio: There's also Python's nice coverage module, which can be easily used with py.test. It shows you which lines of your production code weren't executed by any of the tests.
from lasagne.
Sounds like this could actually be useful to track our progress writing tests for the base library so far. Can this be integrated into our current setup with travis?
from lasagne.
There's coveralls which can be integrated into Travis CI. scikit-learn uses that, for example.
from lasagne.
coveralls looks cool, never tried it. I just checked in something that prints out a coverage report at the end of every test run. Right now it looks pretty messy. ;-)
--------------- coverage: platform linux2, python 2.7.6-final-0 ----------------
Name Stmts Miss Cover Missing
------------------------------------------------------------------
nntools/__init__ 7 0 100%
nntools/init 49 2 96% 17, 44
nntools/layers/__init__ 3 0 100%
nntools/layers/base 432 212 51% 59, 337, 366, 371, 391, 394, 407-408, 429, 500-520, 523-524, 527, 530, 533-542, 547-570, 577-597, 600-601, 604, 607, 610-622, 627-651, 660-662, 665-674, 677, 697-704, 708-710, 713-724, 741-747, 751-773, 787-806, 809, 812, 815, 818-832, 840-841, 844, 847, 854, 857, 864-867, 870-877, 880, 893-896, 933-936, 940-942
nntools/layers/corrmm 68 51 25% 26-65, 68-69, 72, 75, 78-82, 85-100
nntools/layers/cuda_convnet 174 140 20% 25-80, 83-88, 91, 94, 97-110, 113-138, 143-164, 167-182, 185-194, 209, 212, 223, 226, 241-260, 263, 266, 269, 272-284
nntools/nonlinearities 10 2 80% 22, 26
nntools/objectives 15 0 100%
nntools/regularization 8 4 50% 8-13
nntools/tests/conftest 6 0 100%
nntools/tests/test_examples 24 9 63% 22-23, 29-36
nntools/tests/test_init 39 0 100%
nntools/tests/test_layers 227 0 100%
nntools/tests/test_objectives 36 0 100%
nntools/theano_extensions/__init__ 0 0 100%
nntools/theano_extensions/conv 113 104 8% 17-35, 42-56, 63-77, 86-116, 123-170, 177-211
nntools/theano_extensions/padding 20 16 20% 16-38
nntools/updates 60 51 15% 12-18, 22-31, 37-48, 56-65, 76-85, 97-115
nntools/utils 45 9 80% 20-21, 35-36, 48-51, 98, 108
------------------------------------------------------------------
TOTAL 1336 600 55%
from lasagne.
I think our policy has been pretty much decided, we want pull requests to come with tests as much as possible (and if they don't we should encourage the submitter to add them, or add them ourselves before merging). So I'll close this issue.
The first step to implementing this policy is getting our test coverage up to an acceptable level, so contributors will be encouraged to write tests themselves as well. I made a new issue for this: #112.
from lasagne.
Related Issues (20)
- ThinPlateSpline is bugged, proposed fix HOT 2
- Update Lasagne installation doc to new gpuarray backend HOT 3
- AttributeError: 'Conv2DLayer' object has no attribute 'flip_filters' HOT 1
- Theano discontinuation HOT 5
- Where is the GlobalMaxPool2D?? HOT 1
- How to save layer l_out as lasagne layer to network in json or h5 format to be imported from Matlab HOT 1
- how to get the exact value of the tensor variable and its type. HOT 2
- The tremendous different time consuming on mnist between cnn and mlp architecture. HOT 6
- How to put constraint on the weights in each layer. HOT 1
- How to put the constraint on parameters
- AttributeError: 'Conv2DLayer' object has no attribute 'num_groups' HOT 2
- Why the `bcast` is needed in `create_param()`? HOT 2
- rules in setting weights in the combination of conv2d layer and batch norm layer HOT 1
- updates.py HOT 2
- Hi! There are some problems about creating a new layer! HOT 1
- lasagne\layers\base.py HOT 1
- LocallyConnected2DLayer params not initialized correctly HOT 1
- Center Loss as an Objective Function?
- Error with mock in Python 3.8.3 and 3.9 HOT 3
- lasagne isn't running on CUDA (Windows 10) .theanorc setup HOT 2
Recommend Projects
-
React
A declarative, efficient, and flexible JavaScript library for building user interfaces.
-
Vue.js
🖖 Vue.js is a progressive, incrementally-adoptable JavaScript framework for building UI on the web.
-
Typescript
TypeScript is a superset of JavaScript that compiles to clean JavaScript output.
-
TensorFlow
An Open Source Machine Learning Framework for Everyone
-
Django
The Web framework for perfectionists with deadlines.
-
Laravel
A PHP framework for web artisans
-
D3
Bring data to life with SVG, Canvas and HTML. 📊📈🎉
-
Recommend Topics
-
javascript
JavaScript (JS) is a lightweight interpreted programming language with first-class functions.
-
web
Some thing interesting about web. New door for the world.
-
server
A server is a program made to process requests and deliver data to clients.
-
Machine learning
Machine learning is a way of modeling and interpreting data that allows a piece of software to respond intelligently.
-
Visualization
Some thing interesting about visualization, use data art
-
Game
Some thing interesting about game, make everyone happy.
Recommend Org
-
Facebook
We are working to build community through open source technology. NB: members must have two-factor auth.
-
Microsoft
Open source projects and samples from Microsoft.
-
Google
Google ❤️ Open Source for everyone.
-
Alibaba
Alibaba Open Source for everyone
-
D3
Data-Driven Documents codes.
-
Tencent
China tencent open source team.
from lasagne.