Migrate Utility functions to core #2406
Draft
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
egede
temporarily deployed
to
Integrate Pull Request
GitHub Actions
Inactive
|
@Ishaan-Chandak I just created a PR from your branch as it makes it easier to comment on the code. |
egede
commented
Mar 13, 2025
There was a problem hiding this comment.
Try to implement the changes suggested here. If that then removes the warnings, then the entire code base should be changed to use the new import location.
It would be good to write test cases for all the different utility functions as well. If you decide to do this, the tests should utilise the mock system to avoid actually submitting and running any jobs as part of the tests.
|
Another comment is that we should make sure that the old import location still works but emits a deprecation warning. While the code is not formally part of the Ganga API, it will have a number of external users. |
Ishaan-Chandak
temporarily deployed
to
Integrate Pull Request
GitHub Actions
Inactive
egede
commented
Mar 13, 2025
ganga/ganga/__init__.py
Outdated
| @@ -10,6 +10,7 @@ | |||
| import GangaCore.Core | |||
| from GangaCore.Core.GangaRepository import getRegistry | |||
| from GangaCore.Core.InternalServices.ShutdownManager import _protected_ganga_exitfuncs | |||
| from ganga.GangaCore.Utility.Config.Config import ConfigError | |||
There was a problem hiding this comment.
Is ConfigError ever used? If not, we can delete this line.
There was a problem hiding this comment.
no it was not being used,
I have deleted this
Ishaan-Chandak
requested a deployment
to
Integrate Pull Request
GitHub Actions
Waiting
|
OK, so I guess the final thing that is required is to change all the imports in the testing code. I give a list below. I think a global search and replace with the new location should work. |
egede
commented
Mar 13, 2025
ganga/GangaTest/Framework/utils.py
Outdated
Comment on lines
4
to
9
| warnings.warn( | ||
| "The module 'ganga.GangaTest.Framework.util' is deprecated and will be removed in a future release. " | ||
| "Please use 'ganga.GangaCore.Utility.job_monitoring' instead.", | ||
| DeprecationWarning, | ||
| stacklevel=2 | ||
| ) |
There was a problem hiding this comment.
Did you check that this is actually working
There was a problem hiding this comment.
Yes, I did test this. I tested the read_file functionality. I created a python script and a dummy read file and checked by importing the read_file function from GangaTest.framework.utils.
It did work correctly.
Ishaan-Chandak
requested a deployment
to
Integrate Pull Request
GitHub Actions
Waiting
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
No description provided.