Skip to content

Comments

Handle spaces in multiple file property#40767

Draft
samtygier-stfc wants to merge 4 commits intomainfrom
40648-handle-spaces-in-MultipleFileProperty
Draft

Handle spaces in multiple file property#40767
samtygier-stfc wants to merge 4 commits intomainfrom
40648-handle-spaces-in-MultipleFileProperty

Conversation

@samtygier-stfc
Copy link
Contributor

@samtygier-stfc samtygier-stfc commented Jan 29, 2026

Description of work

DO NOT MERGE yet, still has debugging code

This changes the regular expression used to separate groups of filenames to be more robust against spaces after commas.

It results in POLARIS146554, 146603-146609, 146616 being treated the same as POLARIS146554,146603-146609,146616. This is a change from the current behaviour which would treat it as POLARIS146554, XXX146603-146609, XXX146616 where XXX is the current default instrument.

The previous regex used to find the commas that separate groups of files from separate instruments:
NUM_COMMA_ALPHA(R"((?<=\d)\s*,\s*(?=\D))")
uses a lookahead with \D to find a comma followed by something that did not start with a digit. E.g. this would match the comma in ,ALF123 but not ,123. The bug is that it also matches , 123 because the space is a non-digit character. Because we now have a group with a filename that starts with a number, the default instrument is assumed.

This fix replaces \D with [a-zA-Z] so that a new group is only started if there is a new instrument name.

Related to #40648
Closes #40793

To test:


Reviewer

Your comments will be used as part of the gatekeeper process. Comment clearly on what you have checked and tested during your review. Provide an audit trail for any changes requested.

As per the review guidelines:

  • Is the code of an acceptable quality? (Code standards/GUI standards)
  • Has a thorough functional test been performed? Do the changes handle unexpected input/situations?
  • Are appropriately scoped unit and/or system tests provided?
  • Do the release notes conform to the guidelines and describe the changes appropriately?
  • Has the relevant (user and developer) documentation been added/updated?
  • If the PR author isn’t in the mantid-developers or mantid-contributors teams, add a review comment rerun ci to authorize/rerun the CI

Gatekeeper

As per the gatekeeping guidelines:

  • Has a thorough first line review been conducted, including functional testing?
  • At a high-level, is the code quality sufficient?
  • Are the base, milestone and labels correct?

@samtygier-stfc samtygier-stfc force-pushed the 40648-handle-spaces-in-MultipleFileProperty branch from a13695d to 46670c3 Compare January 30, 2026 09:31
@samtygier-stfc
Copy link
Contributor Author

There are a few test failures. This will need to handle cases like:
LagrangeILLReduction(SampleRuns="014412.nxs, 014412_scan_continues.nxs")

@samtygier-stfc samtygier-stfc added ISIS: Core Issue and pull requests at ISIS that relate to Core features and functions Maintenance Unassigned issues to be addressed in the next maintenance period. labels Feb 5, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ISIS: Core Issue and pull requests at ISIS that relate to Core features and functions Maintenance Unassigned issues to be addressed in the next maintenance period.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Improve parsing of filenames for Load to be more robust around whitespace

1 participant