Ticket #10836 (closed: fixed)
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: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: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: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: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
Re #10836 Tests which outline and illustrate the behavior desired
Changeset: 1e239be449b876b05d787507084597df0f0268c2