ioam / topographica

A general-purpose neural simulator focusing on topographic maps.
topographica.org
BSD 3-Clause "New" or "Revised" License
53 stars 32 forks source link

Tab completion when defining submodel parameters #574

Closed jlstevens closed 10 years ago

jlstevens commented 10 years ago

This Issue has been spun out of #573 which started as a general discussion about the new submodel machinery.

In our current design, parameters are specified by kwarg dictionaries returned by appropriate decorated methods. The issue is that the keys are currently manually specified as strings although all the appropriate parameters are already available at the class level. Writing strings requires remembering parameter names exactly and is error prone.

This situation could be improved by offering IPython tab-completion for the valid parameter names. I'll make a first pass at a possible solution in my next post below.

jlstevens commented 10 years ago

I think I have a reasonable solution to tab-completion. The main obstacle is that IPython needs access to an object to tab-complete it so anything generated at run-time cannot be tab-completed.

    @Model.level('Retina', sheet.GeneratorSheet)
    def photoreceptor_sheet_parameters(self, properties):
      ...

    @Model.level('LGN', sheet.CFSheet)
    def DoG_sheet_parameters(self, properties):
      ...

The information about the sheet types (and therefore tab-completable parameters) is available at definition time (when decorators are executed). This means tab-completion should be possible!

    @generatorsheet('Retina')
    def photoreceptor_sheet_parameters(self, properties):
      ...

    @cfsheet('V1')
    def DoG_sheet_parameters(self, properties):
      ...

To summarize, I think it would be a general improvement (with tab-completion) if we defined sheet parameters with this style:

    @generatorsheet('Retina')
    def photoreceptor_sheet_parameters(self, properties):
        return {generatorsheet.period:1.0,
                   generatorsheet.phase:0.05,
                   ...
                  }

Is this approach an improvement?

jlstevens commented 10 years ago

I've talked to Tobias and everything I wrote above can be made to apply the projections.

The only reason things haven't been consistent between sheets and projections is that internally topo.sim.connect has been used to define projections. The connect method is just syntactic sugar for creating Projection objects which is what we should be doing here.

This way, the projection type will be declared in the same way as the sheet type (in the decorator) instead of being a parameter returned by the method for connect to use.

I will now work on making handling of projections and sheets consistent which makes sense without needing to agree with the above suggestion.

EDIT: I've just looked at the code and realized there is no point removing the call to topo.connect. The connection decorator is already receiving the projection type so it is effectively what I have just syggested. This means projections could trivially use exactly the same system as proposed above (i.e. we could tab-complete projection parameters).

jlstevens commented 10 years ago

Here is an even better suggestion. Just add this method to the decorators:

    def settings(self, **settings):
        return settings

Now by dynamically setting the docstring on this method, IPython will offer tab completion without needing any other magic. Defining parameters would now be even more concise. The example above would now be:

    @generatorsheet('Retina')
    def photoreceptor_sheet_parameters(self, properties):
        return generatorsheet.settings(period=1.0, phase=0.05)

Of course, there may be a more suitable name for the method than 'settings' but it is the best I could come up with for now (maybe 'kwargs' could also work?)

Anyway, nothing stops you using a regular dictionary but I would personally much prefer this style. How could anyone say no to tab completion? :-)

jbednar commented 10 years ago

I can't follow all the nuances, but it all sounds good to me...

jlstevens commented 10 years ago

Ok. I'll commit this soon and if anyone has any major objections I'm sure I'll soon know about it!

jlstevens commented 10 years ago

Committed in 8a856f7 and 00ec4e6

I would still like a nicer name than 'settings' and I am sure my imports for registering a default set of sheets and projections could be cleaned up a bit.

jlstevens commented 10 years ago

As the decorator is not a parameterized object, I could use the name 'params' instead of 'settings'. Would this make sense or would it just be confusing?

jlstevens commented 10 years ago

As this feature is committed and working, I'll close this Issue. Anyone is still free to add new comments of course...

jbednar commented 10 years ago

Calling it params makes sense to me; aren't they always Parameters?