Skip to content

Conversation

@clevelam
Copy link
Collaborator

@clevelam clevelam commented Jul 29, 2025

  • Modernize to .toml file
  • Use TempoaryDirectory to prevent clutter during testing
  • Fix multiple arrival times plotting
  • Update version number and change log to opppy-0_1_13

@clevelam
Copy link
Collaborator Author

@tylerjereddy do you want to take a look at this for me? I had a hard time figuring out how to use tmp_path so I opted to use TempoaryDirectory, do you think this is an alright solution?

Copy link
Contributor

@tylerjereddy tylerjereddy left a comment

Choose a reason for hiding this comment

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

I made a small suggestion about constructing the output paths a bit more robustly, but I did confirm that this branch avoids polluting the user directory when running the testsuite.

I do have other things to say of course, like it would be great to avoid the os.system() testing business for a plethora of reasons, and probably switching away from unittest.TestCase to conventional pytest tests so we could more easily use fixtures. But that's a larger endeavor, and this is at least a little cleaner than before.

@clevelam
Copy link
Collaborator Author

I made a small suggestion about constructing the output paths a bit more robustly, but I did confirm that this branch avoids polluting the user directory when running the testsuite.

I do have other things to say of course, like it would be great to avoid the os.system() testing business for a plethora of reasons, and probably switching away from unittest.TestCase to conventional pytest tests so we could more easily use fixtures. But that's a larger endeavor, and this is at least a little cleaner than before.

Yeah, I don't particularly care for the os.system calls either. I chose this because it was the only way I could think of to emulate the interactive nature of the parser (as used on the command line by users).

@clevelam clevelam merged commit ef39272 into lanl:master Aug 27, 2025
6 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.

2 participants