Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Remove dependence on FilteredTimeSeriesProperty from ISISRunLogs #34927

Closed

Conversation

peterfpeterson
Copy link
Member

@peterfpeterson peterfpeterson commented Jan 12, 2023

As part of #34794 this starts removing dependency on FilteredTimeSeriesProperty which is only used by one algorithm, ISSRunLogs. The change is to filter the logs the same way as everywhere else currently in mantid by removing and adding values for the various windows.

To test:

This is a refactor that should not break any existing tests.

There is no associated issue.

This does not require release notes because it is a refactor of underlying code.


Reviewer

Please comment on the following (full description):

Code Review
  • Is the code of an acceptable quality?
  • Does the code conform to the coding standards?
  • Are the unit tests small and test the class in isolation?
  • If there is GUI work does it follow the GUI standards?
  • If there are changes in the release notes then do they describe the changes appropriately?
  • Are the release notes saved in a separate file, using Issue or PR number for file name and in the correct location?
Functional Tests
  • Do changes function as described? Add comments below that describe the tests performed?
  • Do the changes handle unexpected situations, e.g. bad input?
  • Has the relevant (user and developer) documentation been added/updated?

Does everything look good? Mark the review as Approve. A member of @mantidproject/gatekeepers will take care of it.

@peterfpeterson peterfpeterson added this to the Release 6.6 milestone Jan 12, 2023
@peterfpeterson peterfpeterson marked this pull request as ready for review January 12, 2023 19:11
@ckendrick ckendrick self-requested a review January 13, 2023 15:12
@github-actions
Copy link

👋 Hi, @peterfpeterson,

Conflicts have been detected against the base branch. Please rebase your branch against the base branch.

@github-actions github-actions bot added the Has Conflicts Used by the bot to label pull requests that have conflicts label Jan 18, 2023
@ckendrick ckendrick force-pushed the 928_isisrunlogs_alg branch from 7fdf933 to 2418fa3 Compare January 18, 2023 20:57
@github-actions github-actions bot removed the Has Conflicts Used by the bot to label pull requests that have conflicts label Jan 18, 2023
@github-actions github-actions bot added the Has Conflicts Used by the bot to label pull requests that have conflicts label Jan 23, 2023
@github-actions
Copy link

👋 Hi, @peterfpeterson,

Conflicts have been detected against the base branch. Please rebase your branch against the base branch.

# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
Has Conflicts Used by the bot to label pull requests that have conflicts
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants