simphony / simphony-mayavi

The mayavi adapters to the simphony framework
BSD 2-Clause "Simplified" License
0 stars 1 forks source link

EngineFactory class #130

Closed kitchoi closed 8 years ago

kitchoi commented 8 years ago

User experience would be similar

Before (master): old

After (this branch): new new2

kitchoi commented 8 years ago

@itziakos How about this? :)

itziakos commented 8 years ago
kitchoi commented 8 years ago

Mmmmmm I thought that was desired.

itziakos commented 8 years ago

UI: It is not necessary to use the word factory in the ui labels. Just Engines or Registered Engines should be enough.

kitchoi commented 8 years ago

It would be more elegant to simply name "registered engine", but I personally think the word "create from factory" highlights the fact that a new instance is being created. I am not sure how obvious the implication would be without explicitly saying so.

kitchoi commented 8 years ago

On second thought, maybe "factory" means nothing to non-developers. Ah I am biased, so I don't know. I think you're right. But I also think the word "create" should be kept.

kitchoi commented 8 years ago

This is how it looks now. renamed

I think we should keep the "load from *.py" for now because it is useful (and it is in the master already). To use the Mayavi's "run script" function and retrieve a local variable from there needs some more work.

kitchoi commented 8 years ago

actually, I couldn't do meaningful tests for the engine factory api because (1) i would be patching the non-existing modules with ones that most definitely create a ABCModelingEngine instance. (2) stevedore extensions is hard to fake, it may be possible, but I haven't found out how.

kitchoi commented 8 years ago

I think at the moment I'd better move on to other issues. Please let me know if this is good to merge, @itziakos

kitchoi commented 8 years ago

@itziakos, could this be merged and I implement the removal of load-from-py #138 in a separate PR?