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

Deleting all legacy PACKAGE.py #2247

Merged
merged 53 commits into from Apr 15, 2024
Merged

Deleting all legacy PACKAGE.py #2247

merged 53 commits into from Apr 15, 2024

Conversation

souravpy
Copy link
Contributor

@souravpy souravpy commented Jan 2, 2024

Deleted following files as remarked

./GangaCore/PACKAGE.py
./GangaDirac/PACKAGE.py
./GangaGaudi/PACKAGE.py
./GangaLHCb/PACKAGE.py
./GangaND280/PACKAGE.py
./GangaTest/Lib/GListApp/PACKAGE.py
./GangaTest/Lib/TFile/PACKAGE.py
./GangaTest/PACKAGE.py
./GangaTutorial/PACKAGE.py

@souravpy souravpy had a problem deploying to Integrate Pull Request January 2, 2024 08:34 — with GitHub Actions Failure
@souravpy souravpy changed the title Deleting all legacy PACKAGE.py refrencing https://github.com/ganga-devs/ganga/issues/2221#issue-2030029170 Deleting all legacy PACKAGE.py Jan 2, 2024
mesmith75
mesmith75 previously approved these changes Jan 2, 2024
@souravpy souravpy temporarily deployed to Integrate Pull Request January 2, 2024 13:59 — with GitHub Actions Inactive
@egede
Copy link
Member

egede commented Jan 9, 2024

Some tests are failing, reporting a failure of import GangaCore.PACKAGE. It should be investigated if those lines can just be deleted.

@egede
Copy link
Member

egede commented Jan 9, 2024

The PackageSetup class should be deleted as well.

Removed PackageSetup class
@souravpy souravpy had a problem deploying to Integrate Pull Request January 9, 2024 15:43 — with GitHub Actions Failure
@souravpy
Copy link
Contributor Author

souravpy commented Jan 9, 2024

The PackageSetup class should be deleted as well.

Done as suggested, kindly check the PR

Removed GangaCore PACKAGE.py dependency
@souravpy souravpy had a problem deploying to Integrate Pull Request January 9, 2024 16:02 — with GitHub Actions Failure
Removed GangaCore PACKAGE.py dependency
@souravpy souravpy temporarily deployed to Integrate Pull Request January 9, 2024 16:05 — with GitHub Actions Inactive
Removed GangaCore PACKAGE.py dependency
@souravpy souravpy temporarily deployed to Integrate Pull Request January 9, 2024 16:09 — with GitHub Actions Inactive
@egede egede temporarily deployed to Integrate Pull Request February 14, 2024 10:59 — with GitHub Actions Inactive
@souravpy souravpy temporarily deployed to Integrate Pull Request February 24, 2024 17:26 — with GitHub Actions Inactive
@souravpy souravpy temporarily deployed to Integrate Pull Request March 1, 2024 04:05 — with GitHub Actions Inactive
@egede egede temporarily deployed to Integrate Pull Request March 18, 2024 04:20 — with GitHub Actions Inactive
@egede
Copy link
Member

egede commented Mar 18, 2024

We see a set of failures in the testing here, that I assume is a side effect of the code that has been removed.

/home/runner/venv/lib/python3.11/site-packages/coverage/inorout.py:503: CoverageWarning: Module ganga/GangaCore/PACKAGE.py was never imported. (module-not-imported)
  self.warn(f"Module {pkg} was never imported.", slug="module-not-imported")
/home/runner/venv/lib/python3.11/site-packages/coverage/inorout.py:503: CoverageWarning: Module ganga/GangaCore/__init__.py was never imported. (module-not-imported)
  self.warn(f"Module {pkg} was never imported.", slug="module-not-imported")
/home/runner/venv/lib/python3.11/site-packages/coverage/report_core.py:109: CoverageWarning: Couldn't parse Python file '/home/runner/work/ganga/ganga/ganga/GangaCore/Utility/Setup.py' (couldnt-parse)
  coverage._warn(msg, slug="couldnt-parse")
collecting ... collected 139 items / 2 errors

==================================== ERRORS ====================================
_____ ERROR collecting ganga/GangaCore/test/Unit/Notebook/TestNotebook.py ______
ImportError while importing test module '/home/runner/work/ganga/ganga/ganga/GangaCore/test/Unit/Notebook/TestNotebook.py'.
Hint: make sure your test modules/packages have valid Python names.
Traceback:
/opt/hostedtoolcache/Python/3.11.8/x64/lib/python3.11/importlib/__init__.py:126: in import_module
    return _bootstrap._gcd_import(name[level:], package, level)
ganga/GangaCore/test/Unit/Notebook/TestNotebook.py:15: in <module>
    from GangaCore.Lib.Notebook import Notebook
ganga/GangaCore/Lib/Notebook/__init__.py:2: in <module>
    from .Notebook import Notebook
E   ModuleNotFoundError: No module named 'GangaCore.Lib.Notebook.Notebook'

I think the first step is to understand the warinings and the reason for the failure of the import. The nthe fix might actually be easy.

@souravpy souravpy temporarily deployed to Integrate Pull Request March 29, 2024 11:09 — with GitHub Actions Inactive
@mesmith75
Copy link
Contributor

I think this has gone in the wrong direction - we want to keep the Notebook, not remove it.

@souravpy
Copy link
Contributor Author

I think this has gone in the wrong direction - we want to keep the Notebook, not remove it.

I'm keeping it, just omitting the line that is giving error on import, I'll fix it asap!

@mesmith75
Copy link
Contributor

I'm not sure I understand - it gives you the error because you deleted the file. You need to keep the file and that import.

@souravpy souravpy temporarily deployed to Integrate Pull Request March 31, 2024 13:34 — with GitHub Actions Inactive
@egede egede had a problem deploying to Integrate Pull Request April 10, 2024 01:16 — with GitHub Actions Failure
@egede egede temporarily deployed to Integrate Pull Request April 10, 2024 01:16 — with GitHub Actions Inactive
@egede
Copy link
Member

egede commented Apr 10, 2024

@souravpy I fixed the collision caused by the file that was removed/restored. When developing code on a fork, please use a branch rather than the just the main branch (called develop here). In the way you have done it here, you have granted all Ganga developers write access to the main branch in your fork. Probably not what you wanted.

@mesmith75 mesmith75 temporarily deployed to Integrate Pull Request April 15, 2024 09:23 — with GitHub Actions Inactive
@mesmith75 mesmith75 merged commit bd2ee0b into ganga-devs:develop Apr 15, 2024
8 of 10 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants