Ticket #5902 (closed: fixed)

Opened 8 years ago

Last modified 5 years ago

TimeSeriesProperty code review

Reported by: Russell Taylor Owned by: Russell Taylor
Priority: major Milestone: Release 3.2
Component: Framework Keywords: Maintenance
Cc: Blocked By:
Blocking: Tester: Nick Draper

Description (last modified by Russell Taylor) (diff)

I think there is scope for some improvement here.

For example:

  • The valuesAsVector method is rather expensive and is even used internally in lots of places in this class.
  • I'm not convinced that the TimeValueUnit struct is necessary. I certainly don't think the operators are.

Change History

comment:1 Changed 8 years ago by Nick Draper

  • Milestone changed from Release 2.3 to Release 2.4

Moved to milestone 2.4

comment:2 Changed 8 years ago by Nick Draper

  • Status changed from new to assigned
  • Owner set to Russell Taylor

comment:3 Changed 8 years ago by Nick Draper

  • Milestone changed from Release 2.4 to Release 2.5

Moved at the code freeze for release 2.4

comment:4 Changed 7 years ago by Russell Taylor

  • Milestone changed from Release 2.5 to Release 2.6

Move the tickets I'm definitely not going to do this iteration so that I can better see the ones that I might.

comment:5 Changed 7 years ago by Nick Draper

  • Status changed from assigned to new

comment:6 Changed 7 years ago by Nick Draper

  • Component changed from Mantid to Framework

comment:7 Changed 7 years ago by Nick Draper

  • Milestone changed from Release 2.6 to Backlog

Moved to backlog at the code freeze for R2.6

comment:8 Changed 7 years ago by Russell Taylor

  • Keywords Maintenance added
  • Description modified (diff)

comment:9 Changed 7 years ago by Russell Taylor

  • Milestone changed from Backlog to Release 3.2

comment:10 Changed 7 years ago by Nick Draper

  • Status changed from new to assigned

Bulk move of tickets out of triage (new) to assigned at the introduction of the triage state

comment:11 Changed 6 years ago by Russell Taylor

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

This ticket is very stale and the TimeSeriesProperty seems to be working fairly well - there are no big performance complaints that I'm aware of. I did take a quick look through and I think there are changes that could be made - some kind of iterator access for example that could take you through the ordered Time/Value pairs without having to copy one or the other out into a new vector. That's not a quick win though.

comment:12 Changed 6 years ago by Nick Draper

  • Status changed from verify to verifying
  • Tester set to Nick Draper

comment:13 Changed 6 years ago by Nick Draper

  • Status changed from verifying to closed

comment:14 Changed 5 years ago by Stuart Campbell

This ticket has been transferred to github issue ​6748

Note: See TracTickets for help on using tickets.