-
Notifications
You must be signed in to change notification settings - Fork 1
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changing user experience #105
Conversation
- Replaced cache and experiments by only one argument: inputs - CLI working as expected
- Introduced modifyied __name__ == "__main__" statement - Also accounts for when the script is run from within the mantid editor
- Previously the path to the experiment directory was defined in terms of scriptName - scriptName was defined in two places, which is not ideal - Defined scriptName in a single place, ICHelpers.py
Mantid editor caches variables in the module scope, leading to wrong behaviours The changes implemented in this commit fix this by calling the variables inside functions
- Changed the way system tests are run by using new functunality - Now tests use inputs from the same file by setting the inputs in mvesuvio config - Bootstrap and Jacknife tests are still ignored, to be corrected in the future
- The previous approach of writing the version to __init__.py overwrites the file - Safer to write version to unique file named _version.py - Don't really know how the versioning works on github so I am still figuring that out
This reverts commit 0f08320.
This reverts commit d4777f0.
- New version of mantid caused libtiff5 to go missing again - Pinned mantid to 6.8 - Updated Ubuntu just because
- Deleted file that was being used to set paths for testing
We really need to get Matthew using conda, as it’s our primary supported installation method! The risk with installing the package this way is that there’s no guarantee that pip won’t break the mantid install, or that the mvesuvio package will work correctly. |
When I run analysis_inputs.py there is a pop-up window in the end asking whether to clean up all the workspaces. Entering
|
The folders are created as expected, by the way, both with the original script and also after renaming. |
Ah, yes, this is intended, although I should probably change this in the future. The first time you run the script, the user should go with 'y' and delete the workspaces that were just loaded into mantid. This will reload the same workspaces but from a local path. In general I put this warning so that the user does not overwrite workspaces accidentally but this is an edge case where this warning should probably not be displayed. |
I've repeated step 6. to 14. with a folder on one drive and it also worked as expected. |
Description of work:
After meeting with Matthew, it was clear that a different way of interacting with the code was needed, as described in #104. The main changes this PR introduces are the following:
analysis_inputs.py
scriptcache
,experiment
andipfolder
to justinputs
andipfolder
. This makes it harder to set up experiments through the terminal but makes it simpler to set up experiments when running ananalysis_inputs.py
script.To test:
conda create -n mvesuvio-test
conda activate mvesuvio-test
mamba install mantidworkbench==6.8
cd
into the repository with this PR and install package by runningpip install -e .
mvesuvio config
to set up default configuration~/.mvesuvio
and copyanalysis_inputs.py
fileanalysis_inputs.py
file inside it.workbench
)analysis_inputs.py
file you just pasted.analysis_inputs
is created alongside the fileanalysis_inputs.py
.analysis_inputs.py
file in the same directory but this time rename it to something differentFixes #104.