Conversation
JeanLucPons
requested review from
TeresiaOlsson,
gubaidulinvadim and
kparasch
|
All should be OK to merge. |
TeresiaOlsson
left a comment
TeresiaOlsson
left a comment
There was a problem hiding this comment.
Only comment I have is that I suggest to not move everything from load_accelerator into accelerator.load() but keep the parts that are file related in fileloader as preparation to later use accelerator.load() to also load from other sources like database etc.
| return load_accelerator(filename, use_fast_loader) | ||
| # Asume that all files are referenced from | ||
| # folder where main AML file is stored | ||
| if not os.path.exists(filename): |
There was a problem hiding this comment.
I like moving the call to the factory in here but I suggest to keep the parts about setting the root folder in the fileloader module since it's something connected to loading from file. That would already prepare for later extending the load function to be able to load from database, web server etc. My suggestion is to add a function load_from_file in fileloader that does the file related parts.
| rootfolder = os.path.abspath(os.path.dirname(filename)) | ||
| set_root_folder(rootfolder) | ||
| config_dict = load(os.path.basename(filename), None, use_fast_loader) | ||
| aml = Factory.depth_first_build(config_dict) |
There was a problem hiding this comment.
I suggest to keep everything up to line 67 and rename the function to load_from_file. We can then later have load_from_database etc which is called from accelerator.load() and use the same factory to build the objects independently of how the config_dict was created.
3 participants
I redo the PR due to too much conflicts that I didn't manage to solve in the previous PR
The aim of this PR is:
int_cs()callignore_externalflag inAccelerator.load()It shows minimum modifications to handle the above and change the way to handle external magnet model and CS backend. This new branch is not compatible with the present Tango backend.
If this PR is accepted then I'll port the Tango backend.
If we decide to limit pyAML to a single control system per
Acceleratorthen this PR is not mandatry and we can change thecontrolsfield tocontroland fix the name tolive.pyaml has been tagged to 0.1.1 and deploed on pyPI.
tango-pyaml has been tagged to 0.3.1 and deployed on pyPI. (pyaml dependencies has been set to accelerator-middle-layer<=0.1.1)
New compatible Tango backend is available in this PR