gidili
commented
5 years ago It looks like the UI is disregarding the fact that some parameters are optional in the schema and possibly disregarding defaults (definitely not being populated if they are there). This is a critical bug and needs to be fixed. We probably missed this because we didn't have any optional parameters in the few classes we tested with before uploading all of them. We should expect more and more things like this to prop-up but we need to make sure.
For dynamically constructed parameters point 4) we had not discussed that case (was not on our radar at all) and it could complicate things significantly requiring some rethinking in the logic, but let's look into it and see if that needs to become a separate issue.
I tried to make a test based on the class "AP1DelayMeanStrongStimTest". Because t_max can be dynamically computed in NeuronUnit from the other parameters (see https://github.com/scidash/neuronunit/blob/dev/neuronunit/tests/base.py#L68), I left it blank. But the site complains that "Missed parameters tmax".
1) That isn't very good English; "Missing parameters: tmax" would be better.
2) According to the schema for the base class (inherited by this test), none of these parameters are actually required: https://github.com/scidash/neuronunit/blob/dev/neuronunit/tests/base.py#L57 because they all have default values: https://github.com/scidash/neuronunit/blob/dev/neuronunit/tests/base.py#L51 So I don't know why it is telling me that I need it. Is the current code just requiring every parameter that appears in the schema? 3) If a user is allowed to skip them, as I would expected for non-required parameters, will the site and database successfully store the default parameters with the test instance? 4) Same as (3), but for dynamically constructed parameters like tmax. There is no test attribute to look up the default value on before the test is constructed. It will only be an attribute after the test is constructed.