ESCOMP / CTSM

Community Terrestrial Systems Model (includes the Community Land Model of CESM)
http://www.cesm.ucar.edu/models/cesm2.0/land/
Other
308 stars 311 forks source link

Adding CTSM python gallery as an external #1985

Closed samsrabin closed 1 year ago

samsrabin commented 1 year ago

As part of my work on testing for #1863, I'm using some Python code that depends on functionality in utils.py in the ctsm_python_gallery repo. For now, I'm just copying over my current version of utils.py and including that. In the future, however, I'm wondering if it would make more sense to include that repo as an external.

ekluzek commented 1 year ago

Yes, it would be easy to add it to the externals. You'll just need to use an explicit hash of the ctsm_python_gallery to get the specific version you need. The question is then where you'd want this to show up in the ctsm checkout. I'm thinking it should go under the python directory. That would allow us to extend the pylint and black functionality in ctsm to cover it as well -- but it wouldn't be setup that way out of the box.

So you would something like this to Externals_CLM.cfg

[ctsm_py_gallery]
local_path = python/ctsm_py_gallery
protocol = git
repo_url = https://github.com/NCAR/ctsm_python_gallery
hash = 6ed2819a0c8801f2868241f83e7851b5ee8e4172
required = True

I'm thinking you might as well set this up now rather than doing it later.

samsrabin commented 1 year ago

Done, thanks!

billsacks commented 1 year ago

Can we discuss this further - either at an upcoming ctsm software / standup meeting or separately? My initial reaction is that, if we have code in CTSM's python library that depends on code in this utils module, we should move some or all of the utils module (maybe the non-plotting pieces of it?) into CTSM's python library rather than bringing in this whole external. We could then have ctsm_python_gallery depend on CTSM's python library rather than the other way around. I think this will make maintenance and further development easier, and it feels better to have the less-tested-and-maintained repository depend on the more-tested-and-maintained repository rather than vice versa. Also, if ctsm_python_gallery does start to get used more, it runs the risk of growing quite large (with all of its python notebooks), which makes it unideal as something to always pull in as an external.

That said, I am open to the plan proposed here, but I just think we shouldn't jump to that without considering some other options for how we might want to do this.

samsrabin commented 1 year ago

Makes sense!

billsacks commented 1 year ago

From discussion at today's ctsm software standup: There is some messiness with bringing in either the python gallery as an external of ctsm or the reverse. For now, let's just copy in the needed parts of utils.py; we'll live with the duplication until it becomes an issue.