Ticket #10836 (closed: fixed)

Opened 6 years ago

Last modified 5 years ago

Modify autoproperties to use standard system dictionary

Reported by: Alex Buts Owned by: Alex Buts
Priority: major Milestone: Release 3.4
Component: Direct Inelastic Keywords:
Cc: Blocked By: #10803
Blocking: #10881 Tester: Ian Bush

Description

When writing Direct ISIS reduction property manager I did not know correct dictionary to place overloaded complex properties so used standard class property dictionary for overloaded properties.

This works but needs custom procedure to write and overload complex properties.

Now I've learned standard pythonic way of overloading such properties. It is reasonable to use such way everywhere, which simplify the code and makes it easy to maintain by anybody with advanced python knowledge.

As almost all complex properties are unit tested now, the changes looks pretty simple.

Change History

comment:1 Changed 6 years ago by Alex Buts

Re #10836 Tests which outline and illustrate the behavior desired

Changeset: 1e239be449b876b05d787507084597df0f0268c2

comment:2 Changed 6 years ago by Alex Buts

Re #10836 A bit more testing of the concept

Changeset: 585915db7997e93ebabf918181f64bd4b96ddc34

comment:3 Changed 6 years ago by Alex Buts

Re #10836 something broken

Changeset: 40e9d7372ad858ae69d1300ec480cc49934ea1c9

comment:4 Changed 6 years ago by Alex Buts

Re #10836 fixed something but not all

Changeset: 68cbe6bbdc80e6fe406af078a4026a85d5952a5e

comment:5 Changed 6 years ago by Alex Buts

Re #10836 Changes outlined

Changeset: 75385a9ae7d00065bf12614a8e153c4fd0636c44

comment:6 Changed 6 years ago by Alex Buts

Re #10836 Fixed most of the stuff

Changeset: 4ad012a22021990076a4503ff680667a8d70adbe

comment:7 Changed 6 years ago by Alex Buts

Re #10836 Changed update from instrument function

Changeset: 58d3f775e8b2bf21f560166ae9a2d747f81b4201

comment:8 Changed 6 years ago by Alex Buts

Re #10836 further test fixing

Changeset: 541b8266d600dfd308941d40721884825ae0c7c3

comment:9 Changed 6 years ago by Alex Buts

Re #10836 Further fixing tests

Changeset: 84a161ebe4a480e57c7fcc266a65690fa8ab9fb3

comment:10 Changed 6 years ago by Alex Buts

Re #10836 Proper accessor to descriptors

Changeset: 88ed06980dd4eb880aedfbda5ef4f97ebdc5e31c

comment:11 Changed 6 years ago by Alex Buts

Re #10836 Minor test for descriptor accessing

Changeset: c1680609c73bf5dd477f820047e1f477981b9fa4

comment:12 Changed 6 years ago by Alex Buts

Re #10836 Comments and removed redundant code

Changeset: d91f12e432859e7f6023e969a01f950f03f1292d

comment:13 Changed 6 years ago by Alex Buts

Re #10836 Refactored RestoreParamFromInstrument

Changeset: 538714ae7e9aba7f3c2176e14d4bee6939a45d20

comment:14 Changed 6 years ago by Alex Buts

Re #10836 Simplified update_defaults_from_instrument and it works

much simple and better then the previous complex solution

Changeset: 09b0770ce8b8c61e047ebb57800684b75a647aa7

comment:15 Changed 6 years ago by Alex Buts

Re #10836 Comments and some clean-up for the code

Changeset: cb5655d4b1a3139557a26a055577c47d4212067a

comment:16 Changed 6 years ago by Alex Buts

Re #10836 minor bug in HardMaskOnly implementation

Changeset: 3e342f7051076c363c6a956a31a2014e0f1ec0af

comment:17 Changed 6 years ago by Alex Buts

  • Status changed from new to assigned

This is refactoring of Direct inelastic package internal implementation (interface has not been touched) using descriptors and number of standard tricks, I've learned while implementing it in the first place.

All previous functionality present in this class has been unit tested, so as tests pass, one may believe the changes are correct.

No much actual testing is needed. Test by code review.

comment:18 Changed 6 years ago by Alex Buts

  • Status changed from assigned to inprogress

Re #10836 Comments and more descriptions

Changeset: 463eef69d80f2725e66fa6070dc7129db60ef14a

comment:19 Changed 6 years ago by Alex Buts

Re #10836 Trying to fix merge problems

Changeset: f3e3d3f06b3b4ff76922c650fd2b684ea3582014

comment:20 Changed 6 years ago by Alex Buts

  • Blocked By 10803 removed

comment:21 Changed 6 years ago by Alex Buts

Following commits do not carry new information but just do rebasing to current state of #10803

comment:22 Changed 6 years ago by Alex Buts

Re #10836 Tests which outline and illustrate the behavior desired

Changeset: 547a3036440fa6803523f9f221a5875607108055

comment:23 Changed 6 years ago by Alex Buts

Re #10836 A bit more testing of the concept

Changeset: fac03bf49a203ff06e81b270338560915ccc6a78

comment:24 Changed 6 years ago by Alex Buts

Re #10836 something broken

Changeset: 4b15ef74b0996eed07fd558b9dc6c6a28c1ec5e9

comment:25 Changed 6 years ago by Alex Buts

Re #10836 fixed something but not all

Changeset: 4db7c35da80f6da99d9487647acc2db0df7e3acf

comment:26 Changed 6 years ago by Alex Buts

Re #10836 Changes outlined

Changeset: 46ee9639bdcb01b172b2d7c5dde71f65c33ed49b

comment:27 Changed 6 years ago by Alex Buts

Re #10836 Fixed most of the stuff

Changeset: 2196eaf0abcc5059c3e1b91435815ab0daf51a01

comment:28 Changed 6 years ago by Alex Buts

Re #10836 Changed update from instrument function

Changeset: 32a914e48d8a00b4eff355d18e6e20b456800a7a

comment:29 Changed 6 years ago by Alex Buts

Re #10836 further test fixing

Changeset: 1c2c95262a96aaf45b556f82e4664bd183b969d8

comment:30 Changed 6 years ago by Alex Buts

Re #10836 Further fixing tests

Changeset: 9feabe29f9d09988d98cbd6b4e479dc8a79ad5f3

comment:31 Changed 6 years ago by Alex Buts

Re #10836 Proper accessor to descriptors

Changeset: 08bd411e567059883875b4eecb7079ce528050a7

comment:32 Changed 6 years ago by Alex Buts

Re #10836 Minor test for descriptor accessing

Changeset: 4f1e69e9a0f629552929dabac6dbbe3b12ab4534

comment:33 Changed 6 years ago by Alex Buts

Re #10836 Comments and removed redundant code

Changeset: 595e330c4ce80ae853fead82cb1df06dca510529

comment:34 Changed 6 years ago by Alex Buts

Re #10836 Refactored RestoreParamFromInstrument

Changeset: d50ef77ac38d9275532b4e73e7420ace2cb4648a

comment:35 Changed 6 years ago by Alex Buts

Re #10836 Simplified update_defaults_from_instrument and it works

much simple and better then the previous complex solution

Changeset: f5f4ff8bdc1e422a7fdb647e3c2f4778b7c7fd5f

comment:36 Changed 6 years ago by Alex Buts

Re #10836 Comments and some clean-up for the code

Changeset: 235584e6f7dff21d9b9c5a10bec941964c67992d

comment:37 Changed 6 years ago by Alex Buts

Re #10836 minor bug in HardMaskOnly implementation

Changeset: 06980804c87ee0d34fae2a16ccdeb6aaed9efd64

comment:38 Changed 6 years ago by Alex Buts

Re #10836 Comments and more descriptions

Changeset: 75618d1c07b2031fbaacd0998cf231c53ae3bc1a

comment:39 Changed 6 years ago by Alex Buts

Re #10836 Trying to fix merge problems

Changeset: 69058fe664e32944ec8924c7ae1fdc5d5abfe111

comment:40 Changed 6 years ago by Alex Buts

This ends rebase to 10803

comment:41 Changed 6 years ago by Alex Buts

Re #10836 Modified ReductionWrapper to write closing bracket

on separate line. This seems requested by Autoreduction server.

Changeset: 8c8a55093532105aa1dbf488b3b5bded8d77c71b

comment:42 Changed 6 years ago by Alex Buts

Re #10836 Fixed unicode string parsing received from web service

Changeset: 21953687fa11d950333786f5f8c62feb9f0db6d6

comment:43 Changed 6 years ago by Alex Buts

Re #10836 Added method to access DirectEnergyConversion properties

directly, without accessing _propMan explicitly

Changeset: 07fb9c1df121fec35e36cfccbe2cf5e6661d0c97

comment:44 Changed 6 years ago by Alex Buts

Re #10836 Minor changes to DirectEnergyConversion to access properties

directly, without explicitly calling porp_man

Changeset: b4034cded8b475fe92f451b3d3210e6e3cfc4e14

comment:45 Changed 6 years ago by Alex Buts

Re #10836 Test for the overloaded getter - setter

Changeset: 37cdebba1d9a2cd2a97a9b4473995123bc9930ca

comment:46 Changed 6 years ago by Alex Buts

Re #10836 Descriptor for properties with range of values

Class modified to use descriptors for properties with range of values

Changeset: 76f16ebfe43590954a0870a4fcc53c4c1be7e2cf

comment:47 Changed 6 years ago by Alex Buts

Re #10836 Simplified update_defaults_from_instrument logic

and add common PropertyManager interface (returning empty dependencies list by default)

Changeset: 932f44ccc2830e21562e13802437d4fd7d19262f

comment:48 Changed 6 years ago by Alex Buts

  • Status changed from inprogress to verify
  • Resolution set to fixed

This is refactoring ticket, with changes, briefly summarized in PropertyManager.py class file description.

No changes in functionality attempted but number of tests are written to validate the desired behaviour and number of comments added and code improvements attempted.

Test only after #10803 as this is continuation of the work done there.

If tester wands to discuss code style and implementation then please, evaluate the changes. If not -- just merge it to master. This is safe as soon as unit tests and system tests are passing successfully.

comment:49 Changed 6 years ago by Alex Buts

  • Blocked By 10803 added

comment:50 Changed 6 years ago by Alex Buts

  • Blocking 10881 added

comment:51 Changed 6 years ago by Alex Buts

  • Blocking 10684 removed

comment:52 Changed 6 years ago by Alex Buts

Re #10836 Basis for RunDescriptor property

Basis run property class created and tested.

Changeset: 4f257dca8a3e35fa722cff22a64a11db6e504985

comment:53 Changed 6 years ago by Ian Bush

  • Status changed from verify to verifying
  • Tester set to Ian Bush

comment:54 Changed 6 years ago by Alex Buts

  • Blocking 10881 removed

comment:55 Changed 6 years ago by Alex Buts

  • Blocking 10881 added

comment:56 Changed 6 years ago by Ian Bush

  • Status changed from verifying to reopened
  • Resolution fixed deleted

In ReductionWrapperTest.py the PATH appending should probably removed: +os.environ["PATH"] = r"c:/Mantid/Code/builds/br_10803/bin/Release;"+os.environ["PATH"], although this did not cause any issues for me.

Otherwise all looks good and all tests seem to pass.

comment:57 Changed 6 years ago by Alex Buts

  • Status changed from reopened to inprogress

Re #10836 Commenting local set of Mantid DLL search folder

It is a shame it is there at all, but there are no known way of running this script under debugger outside Mantid As the result, the string sometimes slips into main repository.

Changeset: a07caa8103ca4aa96887a00508d8af665572f468

comment:58 Changed 6 years ago by Alex Buts

  • Status changed from inprogress to verify
  • Resolution set to fixed

comment:59 Changed 6 years ago by Ian Bush

  • Status changed from verify to closed

Merge remote-tracking branch 'origin/feature/10836_SystemDictionary'

Full changeset: c7e45c99fd0d199d65968346ba22dbd67372335e

comment:60 Changed 5 years ago by Stuart Campbell

This ticket has been transferred to github issue 11678

Note: See TracTickets for help on using tickets.