Ticket #10023 (closed: fixed)

Opened 6 years ago

Last modified 5 years ago

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:

  1. SavaParameterFile is not showing in the help-pages list of algorithms. Figure out why.
  2. Rename the SaveLocationParameters property to LocationParameters. It's cleaner.
  3. Add a python usage example to the SaveParameterFile documentation.

Change History

comment:1 Changed 6 years ago by Harry Jeffery

  • Status changed from new to assigned

comment:2 Changed 6 years ago by Harry Jeffery

  • Milestone changed from Backlog to Release 3.3

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.

Last edited 6 years ago by Martyn Gigg (previous) (diff)

comment:8 Changed 6 years ago by Martyn Gigg

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.

  1. 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.
  1. 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

Note: See TracTickets for help on using tickets.