Skip to content

Comments

Move sans gui out of framework to mantidqtinterfaces#40878

Open
jhaigh0 wants to merge 4 commits intomantidproject:mainfrom
jhaigh0:37952_move_sans_gui_to_framework
Open

Move sans gui out of framework to mantidqtinterfaces#40878
jhaigh0 wants to merge 4 commits intomantidproject:mainfrom
jhaigh0:37952_move_sans_gui_to_framework

Conversation

@jhaigh0
Copy link
Contributor

@jhaigh0 jhaigh0 commented Feb 11, 2026

Description of work

Move the gui_logic out of scripts/sans to a new mantidqtinterfaces dir.
This probably all needs a bit of a refactor but I'm trying to do the minimal changes possible in this pr.

A few things I want opinions on:

  • 8f76daf The RowEntries model (previously in gui_logic/models) has been refactored out to a file now in sans/data_objects. This is because it was being used by the command interface (in particular the csv parser) so I needed to eliminated the link between scripts and code which was now in mantidqtinterfaces. Not sure if data_objects is the best dir to put it in?
  • c52af98 I don't think it is strictly necessary to move this System test, I'm in two minds about it. I moved it out of framework to qt because it now imports from mantidqtinterfaces, but it does split it up from all the other sans systerm tests.

It might be good to checkout this branch (or user git test-pr to view the file tree in an IDE to get a good sense of where things have moved to. It's hard to follow on github I think.

The goal of this is to eventually remove the qt dependency from the mantid package. There are still a few more places to sort out for this to happen, but this is the majority of it I think.

Closes #37952

To test:

Build testing all tests on all branches https://builds.mantidproject.org/job/build_branch/1474/


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?

@jhaigh0 jhaigh0 added this to the Release 6.16 milestone Feb 11, 2026
@jhaigh0 jhaigh0 added SANS Issues and pull requests related to SANS Maintenance Unassigned issues to be addressed in the next maintenance period. ISIS: Core Issue and pull requests at ISIS that relate to Core features and functions labels Feb 11, 2026
@jhaigh0 jhaigh0 marked this pull request as draft February 11, 2026 10:55
@jhaigh0 jhaigh0 changed the title 37952 move sans gui to framework Move sans gui out of framework to mantidqtinterfaces Feb 11, 2026
@jhaigh0 jhaigh0 marked this pull request as ready for review February 11, 2026 13:21
@MialLewis MialLewis self-requested a review February 12, 2026 10:04
@github-actions github-actions bot added the Has Conflicts Used by the bot to label pull requests that have conflicts label Feb 17, 2026
@github-actions
Copy link
Contributor

👋 Hi, @jhaigh0,

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

@MialLewis MialLewis force-pushed the 37952_move_sans_gui_to_framework branch from c52af98 to 7385ce7 Compare February 18, 2026 09:44
@github-actions github-actions bot removed the Has Conflicts Used by the bot to label pull requests that have conflicts label Feb 18, 2026
Copy link
Contributor

@MialLewis MialLewis left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Have tested the SANS interface via the manual testing instructions, all appears in working order.

The movement of RowEntries and the system test make sense to me.

@cailafinn cailafinn self-assigned this Feb 20, 2026
Copy link
Contributor

@cailafinn cailafinn left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just one comment on the refactored row_entries.py file.



@dataclass(eq=False)
class _UserEntries(object):
Copy link
Contributor

@cailafinn cailafinn Feb 20, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If this file is now a duplicate of the new one in the framework, could this file be deleted and any imports be moved to the one in the framework?

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. SANS Issues and pull requests related to SANS

Projects

None yet

Development

Successfully merging this pull request may close these issues.

The SANS GUI is in the Framework

3 participants