Skip to content

Add task to publish comparison test results#702

Merged
mranst merged 67 commits into
developfrom
feature/mranst/publish_comparisons
May 8, 2026
Merged

Add task to publish comparison test results#702
mranst merged 67 commits into
developfrom
feature/mranst/publish_comparisons

Conversation

@mranst
Copy link
Copy Markdown
Collaborator

@mranst mranst commented Feb 12, 2026

Description

This PR adds a task that will copy relevant text files (namely jedi_log_comparison.txt) and plots generated by comparison suites to another location. This is intended to be used by the CI tests to publish these results to Dataportal. By default, no publish_directory is set, so this task will not do anything unless it is overridden (It also will not be run unless the comparison fails). A pending PR to CI-workflows will enable this feature for tier 2 tests.

Comment thread src/swell/tasks/publish_comparisons.py Outdated
shiklomanov-an
shiklomanov-an previously approved these changes Apr 8, 2026
Copy link
Copy Markdown
Collaborator

@shiklomanov-an shiklomanov-an 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!

Minor comment: Definitely not a required change for this PR, but take a look at Python's built-in pathlib module. It has a lot of quality-of-life improvements for working with system paths. For example, with this:

from pathlib import Path

...you can write this line as something like:

cycle_dir = Path(self.cycle_dir())
publish_location = Path(self.config.publish_directory())
if cycle_dir.is_dir():
    files = cycle_dir.glob("eva/**/*.png")
    out_path = publish_directory / self.__datetime__.string_directory() / self.get_model()
    out_path.mkdir(parents = True, exist_ok = True)

The other potential advantage of using pathlib.Path in more places is that it improves our ability to type-check places that need paths. Right now, we use str annotations for this, but pathlib.Path would be stricter (and therefore help catch more bugs earlier).

Comment thread src/swell/tasks/publish_comparisons.py Outdated
shiklomanov-an
shiklomanov-an previously approved these changes May 7, 2026
Copy link
Copy Markdown
Collaborator

@shiklomanov-an shiklomanov-an left a comment

Choose a reason for hiding this comment

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

Minor suggestion on consistent usage of pathlib. Then, resolve merge conflicts, pass tests, and I think this is good to go.

Copy link
Copy Markdown
Collaborator

@shiklomanov-an shiklomanov-an left a comment

Choose a reason for hiding this comment

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

I would also replace the shutil.copy calls with pathlib.Path.copy, unless there's a good reason not to. But otherwise, I think this looks good, assuming relevant tests pass.

@mranst
Copy link
Copy Markdown
Collaborator Author

mranst commented May 8, 2026

Thanks! Path.copy is python >3.14, I'll keep that in mind for the future

@mranst mranst merged commit 4c2fb12 into develop May 8, 2026
15 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