search

ucgmsim / velocity_modelling

Next-gen velocity modeling tools
1 stars 0 forks source link

Add Velocity Model Plotting Scripts #3

Closed lispandfound closed 1 month ago

lispandfound commented 1 month ago

Adds a new script to plot the domain of a velocity model from either a realisation JSON file, vm params yaml file, or a custom specified domain. Also adds a wiki page and tests.

lispandfound commented 1 month ago

The only three files that matter in this pull request are plot_velocity_model.py, test_plot_velocity_model.py and Plotting-Velocity-Models.md. The rest will disappear from the request once #2 is merged.

lispandfound commented 1 month ago

I'm going to make this a draft PR for the time being while I rethink the separation of the scripts. I think Joel's comments in particular highlight that I have actually got two scripts here instead of one and some major refactoring is required to separate them.

lispandfound commented 1 month ago

Tests and deptry are currently failing because the workflow is an optional dependency that is used when plotting realisations. This is a feature I eventually want but right now sorting out the cyclic dependencies is a nightmare because the workflow depends on a whole lot of dev branches of other repos. Will have a think about to sort that out tomorrow.

lispandfound commented 1 month ago

Ok I have made some changes per the discussion in the stand-up:

  1. Trialing ruff instead of isort + black for linting of imports, black-compatible formatting issues, and additionally numpy doc issues.
  2. I've also removed the realisation plotting code. It totally works and passes tests but requires importing the workflow to run the tests and the workflow is not ready yet. It pulls in dev branches of a bunch of repos that breaks everything. So it's easier to leave out for now and then I'll add the code + tests when workflow is ready.