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

Unit tests for Examples #605

Merged
merged 21 commits into from
Sep 29, 2023
Merged

Conversation

RasmusOrsoe
Copy link
Collaborator

@RasmusOrsoe RasmusOrsoe commented Sep 25, 2023

Adds unit tests for all examples except for 05_pisa.

This is now how unit tests are run.
IceTray-build:

  • All former tests
  • IceTray Examples

Ubuntu-build:

  • All former tests except IceTray dependent ones.
  • All examples except IceTray and PISA dependent examples.

MacOS:

  • All former tests except IceTray dependent ones.

I removed the example unit tests from the MacOS-build because it takes 30 minutes to complete.

Two examples turned out to be broken: train_tito and train_multiclassifier. I fixed the multiclassification example and created an issue for the other. See #606.

closes #594

Copy link
Collaborator

@AMHermansen AMHermansen left a comment

Choose a reason for hiding this comment

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

Looks good, I noticed you changed many of the example scripts to use parse_known_args instead of parse_args, is there any particular reason for this change?

@@ -90,7 +99,7 @@ def main_icecube_upgrade(backend: str) -> None:
"detector", choices=["icecube-86", "icecube-upgrade"]
)

args = parser.parse_args()
args, unknown = parser.parse_known_args()
Copy link
Collaborator

Choose a reason for hiding this comment

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

Any reason why you've changed to use parse_known_args instead of parse_args?

@@ -85,7 +95,7 @@ def load_data() -> None:
"""
)

args = parser.parse_args()
args, unknown = parser.parse_known_args()
Copy link
Collaborator

Choose a reason for hiding this comment

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

Same as before.

@@ -83,7 +93,7 @@ def main() -> None:
"""
)

args = parser.parse_args()
args, unknown = parser.parse_known_args()
Copy link
Collaborator

Choose a reason for hiding this comment

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

Same as before.

@@ -116,7 +126,7 @@ def main() -> None:
"""
)

args = parser.parse_args()
args, unknown = parser.parse_known_args()
Copy link
Collaborator

Choose a reason for hiding this comment

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

Same as before.

@RasmusOrsoe
Copy link
Collaborator Author

Looks good, I noticed you changed many of the example scripts to use parse_known_args instead of parse_args, is there any particular reason for this change?

Thanks! Yes. Without it, the unit tests won't run. I found that when you run pytest python_script.py the name of the python script is passed as an argument to argparse in python_script.py, which is ofc. an unknown argument. I'm not sure why that is, but this simple change solved the issue.

@RasmusOrsoe RasmusOrsoe merged commit cd3311c into graphnet-team:main Sep 29, 2023
12 checks passed
RasmusOrsoe added a commit to RasmusOrsoe/graphnet that referenced this pull request Oct 25, 2023
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.

Create unit tests for all examples (where possible)
2 participants