Skip to content
New issue

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

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

Already on GitHub? Sign in to your account

Fix SampleData module #7287

Merged
merged 2 commits into from
Oct 17, 2023
Merged

Fix SampleData module #7287

merged 2 commits into from
Oct 17, 2023

Conversation

jcfr
Copy link
Member

@jcfr jcfr commented Oct 17, 2023

Restore the passing of the following tests addressing regressions introduced in d3f49ed (BUG: Make more strings translatable) introduced through:

Tests fixed:

  • py_Slicer4Minute
  • py_ShaderProperties
  • py_RSNA2012ProstateDemo
  • py_VolumeRenderingSceneClose
  • py_SubjectHierarchyGenericSelfTest
  • py_SampleData
  • py_SlicerRestoreSceneViewCrashIssue3445
  • py_RSNAVisTutorial
  • py_JRC2013Vis
  • py_AtlasTests

Tested on a local build generated on Ubuntu 20.04

Fix regression introduced in d3f49ed (BUG: Make more strings translatable)
and restore the passing of the following tests:

* py_Slicer4Minute
* py_ShaderProperties
* py_RSNA2012ProstateDemo
* py_VolumeRenderingSceneClose
* py_SubjectHierarchyGenericSelfTest
* py_SlicerRestoreSceneViewCrashIssue3445
* py_RSNAVisTutorial
* py_JRC2013Vis
* py_AtlasTests
Fix regression introduced in d3f49ed (BUG: Make more strings translatable)
where the builtIn category was renamed from "BuiltIn" to "General"
@jcfr jcfr enabled auto-merge (rebase) October 17, 2023 21:48
Copy link
Member

@pieper pieper left a comment

Choose a reason for hiding this comment

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

That looks fine to me. I don't know when BuiltIn was changed to General, but if that's the way it is then yes, this needs to be updated.

@jcfr
Copy link
Member Author

jcfr commented Oct 17, 2023

I don't know when BuiltIn was changed to General,

Thanks for the review. The change from builtIn to General was done in the context of the pull request linked above. For convenience, see this direct link: d3f49ed#diff-fe7a3220b4dec268c9e7d101aefdfaef3e3ede882c9bbc148adf52fce2298f22R507

@pieper
Copy link
Member

pieper commented Oct 17, 2023

This has done in the context of the pull request linked above

Ah, yes, I see, thanks.

Copy link
Contributor

@jamesobutler jamesobutler left a comment

Choose a reason for hiding this comment

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

Changes make sense

@jcfr jcfr merged commit 417e0a2 into Slicer:main Oct 17, 2023
6 checks passed
@jcfr jcfr deleted the fix-sampledata-module branch October 17, 2023 23:59
@jcfr
Copy link
Member Author

jcfr commented Oct 18, 2023

After the integration of this PR, we are now left with one failing test 👌

image

@cpinter
Copy link
Member

cpinter commented Oct 18, 2023

Great! I'll take a quick look at this last test.

@jcfr
Copy link
Member Author

jcfr commented Oct 18, 2023

take a quick look at this last test.

Thanks for helping with this 🙏

The following issue report should help understand how to fix the "problem":

@cpinter

This comment was marked as off-topic.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants