Ticket #10023 (closed: fixed)
Tweak SaveParameterFile
Reported by: | Harry Jeffery | Owned by: | Harry Jeffery |
---|---|---|---|
Priority: | minor | Milestone: | Release 3.3 |
Component: | Framework | Keywords: | |
Cc: | Blocked By: | ||
Blocking: | Tester: | Martyn Gigg |
Description
Some changes suggested by Anders Markvardsen:
- SavaParameterFile is not showing in the help-pages list of algorithms. Figure out why.
- Rename the SaveLocationParameters property to LocationParameters. It's cleaner.
- Add a python usage example to the SaveParameterFile documentation.
Change History
comment:3 Changed 6 years ago by Harry Jeffery
- Status changed from assigned to inprogress
Rename SaveParameterFile's SaveLocationParameters property.
Refs #10023
Changeset: f7a421331e967f350e13bacb680bb829bdc6df14
comment:4 Changed 6 years ago by Harry Jeffery
Add SaveParametersFile python usage example.
Refs #10023
Changeset: e905a7ff85fcb266a66691e32a0741a9c46371e5
comment:5 Changed 6 years ago by Harry Jeffery
Tidy up SaveParametersFile python usage example.
Refs #10023
Changeset: a36c795ff764156548989cc84be53d9c75c96ed6
comment:6 Changed 6 years ago by Harry Jeffery
- Status changed from inprogress to verify
- Resolution set to fixed
The solution to 1. was the missing .. categories directive in the documentation.
It has been added in the usage example changeset.
comment:7 Changed 6 years ago by Martyn Gigg
- Status changed from verify to reopened
- Resolution fixed deleted
Can I suggest another tidy up of the usage example.
The file it saves to is "/tmp...." which is only valid for unix-like systems. It should be more generic. In other cases for saving files we have gone for
filename = os.path.expanduser("~/params.xml")
so that we know the directory is writable on all platforms. Lets update this now so that it is more consistent with other places.
comment:8 Changed 6 years ago by Martyn Gigg
The test also seems to fall over on the 14.04 build server:
http://builds.mantidproject.org/view/Develop%20Builds/job/develop_ubuntu-14.04/347/consoleFull
comment:9 Changed 6 years ago by Harry Jeffery
- Status changed from reopened to inprogress
Fix failing SaveParameterFiles documentation test.
This commit fixes two issues.
- The output file was not being written in a cross-platform friendly way. We now use os.path.expanduser to select a writable file path on any platform.
- The usage example test was failing. This was caused by the faulty assumption that the output of SaveParameterFile is deterministic. The data it outputs is, but the actual ordering of elements in the XML isn't, so checking the raw contents of the XML file was an unreliable testing method. Instead we now rely on the unit test for SaveParameterFile and provide an example of the output to the user (and the pre-existing link to the parameter file wiki page).
Refs #10023
Changeset: 37ba90f665402e59d97679f71eed31360be57f57
comment:10 Changed 6 years ago by Martyn Gigg
I think we should keep the usage example. They are not meant as tests of whether the algorithm is doing the right thing, as you say the unit test will cover that. It is important for a user to be able copy and paste something from the documentation and be able to run it.
The tests around them are to ensure that we haven't broken running this example. I would suggest putting the example back but instead simply check that the file has been written to the appropriate location and print a message saying so. The testoutput check can then be just check if the message was printed correctly.
I would also keep the example output that is already there as I agree it's a nice way of showing what the file might look like.
comment:11 Changed 6 years ago by Harry Jeffery
Re add categories directive to SaveParameterFile docs.
This was accidentally removed in the previous commit.
Refs #10023
Changeset: 5ce0054bc99ee7e9a30cdce69eb73f994515f7a6
comment:12 Changed 6 years ago by Harry Jeffery
Refs #10023. Tweak usage example testing
Changeset: 109696af6a297ac2f6db0b2260fd3eec6061fefc
comment:13 Changed 6 years ago by Harry Jeffery
- Status changed from inprogress to verify
- Resolution set to fixed
comment:14 Changed 6 years ago by Martyn Gigg
- Status changed from verify to verifying
- Tester set to Martyn Gigg
comment:15 Changed 6 years ago by Martyn Gigg
- Status changed from verifying to closed
Merge remote-tracking branch 'origin/feature/10023_tweak_saveparameterfile_algorithm'
Full changeset: 2eb59b2e6ccf5c2bb8551c76d7e9e159e0bb3a3d
comment:16 Changed 5 years ago by Stuart Campbell
This ticket has been transferred to github issue 10865