Update installation instructions

[navidcy]

Some rewording. Hopefully it's an enhancement.

[navidcy]

@ashjbarnes the readme mentions:

It's recommended that you get started with the example notebooks. In this case you'll want to clone this
entire repo so that you have access to the demo folder. This contains some template configuration files
for MOM6, and example notebooks that walk you through the package.

git clone [email protected]:COSIMA/regional-mom6.git
pip install .

Is this the most updated latest recommendation? Why do people need to clone the repository? Just to get the notebook? They can simply download just the notebook, right?

[navidcy]

For example, users can easily download the notebook from

https://regional-mom6.readthedocs.io/en/latest/demo_notebooks/reanalysis-forced.html

There is a button at the top right.

[ashjbarnes]

The reason for the confusion is partly that some of the functionality requires the premade run directory folder. If you just pip install the package, you don't actually get these files. This is maybe because they sit in the main folder and not with the python code? I'm unsure. Any idea @angus-g ?

Anyway, my workaround has been that the .setup_run_dir method takes an argument to point to the location of the regional-mom6 folder on disk, so that it can find these premade run directories. It's a bit clunky! And it was hard to explain in my updates to the install instructions

[navidcy]

The reason for the confusion is partly that some of the functionality requires the premade run directory folder. If you just pip install the package, you don't actually get these files. This is maybe because they sit in the main folder and not with the python code? I'm unsure. Any idea @angus-g ?

OK, I see. Is this premade run directory big in size or just few text files etc?

Anyway, my workaround has been that the .setup_run_dir method takes an argument to point to the location of the regional-mom6 folder on disk, so that it can find these premade run directories. It's a bit clunky! And it was hard to explain in my updates to the install instructions

Hm... well if that's what needed we need to explain it. Or give a step-by-step instructions.

[ashjbarnes]

No the

The reason for the confusion is partly that some of the functionality requires the premade run directory folder. If you just pip install the package, you don't actually get these files. This is maybe because they sit in the main folder and not with the python code? I'm unsure. Any idea @angus-g ?

OK, I see. Is this premade run directory big in size or just few text files etc?

Anyway, my workaround has been that the .setup_run_dir method takes an argument to point to the location of the regional-mom6 folder on disk, so that it can find these premade run directories. It's a bit clunky! And it was hard to explain in my updates to the install instructions

Hm... well if that's what needed we need to explain it. Or give a step-by-step instructions.

No it's a really small size on disk. I think ideally the pip install of the package should also pull down these premade run directories to keep things simple but I'm not sure how to do this

[navidcy]

No it's a really small size on disk. I think ideally the pip install of the package should also pull down these premade run directories to keep things simple but I'm not sure how to do this

But where would pip install save that directory? Wherever it saves the source code of the package? Or does this directory needs to be wherever the user wants to run the experiment?

[ashjbarnes]

No it's a really small size on disk. I think ideally the pip install of the package should also pull down these premade run directories to keep things simple but I'm not sure how to do this

But where would pip install save that directory? Wherever it saves the source code of the package? Or does this directory needs to be wherever the user wants to run the experiment?

An old implementation of this was to use a relative path from the regional-mom6.py file. This was something like Path(file).parent / "demos". That works when you git clone the repository and then import from there, but doesn't work if you pip install (currently) because the demos folder isn't included. However, if we could guarantee that the files were included in the pip install, we could hardcode the correct relative path again in the setup_rundir function

[navidcy]

As a user, do I need to have this premade_run_dir where I would run my experiment?

[ashjbarnes]

As a user, do I need to have this premade_run_dir where I would run my experiment?

no. the setup_run_dir just needs to know where to find the premade_run_dirs folder, which currently requires it to be git cloned because pip doesn't download it

[navidcy]

gotcha!

[angus-g]

See https://github.com/COSIMA/regional-mom6/tree/angus-g/package-data-install for how to include the demos in the package wheel.

[navidcy]

So @angus-g, if we do what f064842 suggests then this issue with the "premade_run_dir" that @ashjbarnes mentions above is taken care of?

[angus-g]

So @angus-g, if we do what f064842 suggests then this issue with the "premade_run_dir" that @ashjbarnes mentions above is taken care of?

Right, then from within the module code, Path(__file__) / "premade_run_dir" will exist and can be used as desired.

[navidcy]

That sounds good!

OK, I did it. @ashjbarnes try this whenever and if it works fine then we can simplify the installation instructions!

[ashjbarnes]

As it was, installing via pip install . in the module directory ran into a file not found error when trying to copy the premade run directories. I've used the importlib.resources module to return the absolute path to where the package is installed on the computer. This is used for finding the premade run directories

It also checks for the existence of this folder. If it can't find it, then it prompts the user to install via a clone of the repo so that the demo material is present

[angus-g]

Only thing I can think is that importlib.resources is picking up regional_mom6 from the current directory, i.e. the one inside the repository which doesn't have the demos subdir... Because of the way it's set up, you should be able to ask for importlib.resources("regional_mom6.demos.premade_run_dirs"), which will probably make this show up more obviously. I don't know off the top of my head how to make Python not look in the current directory when importing, either, probably not too hard to fix.

[ashjbarnes]

sorry for all the commits... I haven't found a good way to test the workflow locally yet. I've tried to do what @angus-g suggested but it just says that there's 'no such module'.

[navidcy]

Feel free to commit as much as you like!!
We'll squash all the commits into one when we merge

[ashjbarnes]

Ok so the solution was to explicitly include all of the subdirectories of premade_run_directories. I'm still perplexed why doing a fresh install on a new conda env on gadi worked, but not within the docker container on github actions. Anyway I guess this is more robust!

[ashjbarnes]

I've updated the install instructions so they no longer imply the need to git clone and then install to access the premade rundirs

[navidcy]

Ok. Great!

good job figuring it out!!

[navidcy]

It’d be nice if @angus-g’s pair of eyes had a look at this before we merge.

[angus-g]

Looks pretty good. I guess there's still the outstanding change to avoid recommending a clone. Otherwise, just a few places where os.path should be replaced by pathlib directly.

[angus-g]

Ah, nice catch. I think this probably depended on the setuptools version, but this is definitely more robust!

[angus-g]

Why not:

Suggested change

	if surface_forcing != False:
	if surface_forcing:

[navidcy]

Good catch @angus-g. Hm... @ashjbarnes, by writing if surface_forcing != False will execute if surface_forcing = nothing or surface_forcing = "silly_forcing". Was this intentional?

[navidcy]

OK, @angus-g I see that how this method is called surface_forcing could be indeed a string "era5".
So what @ashjbarnes did here is intentional.

It is prone for mistakes though since, at least myself, when I see a variable that can be False I assume it's a boolean so True or False are the only allowed values. I'll open an issue and we can discuss there and fix in a different PR.

[navidcy]

see #134

[angus-g]

I think in that case it should be a string value or None.