NCAR / ADF

A unified collection of python scripts used to generate standard plots from CAM outputs.
Creative Commons Attribution 4.0 International
34 stars 29 forks source link

Dataclass fixes #317

Closed justin-richling closed 2 weeks ago

justin-richling commented 1 month ago

Fix issues from recent data class PR #269:

adf_info.py

global_latlon_map.py

zonal_mean.py

adf_dataset.py

tape_recorder.py:

justin-richling commented 1 month ago

@brianpm I needed to make some adjustments to the methods in adf_dataset.py, let me know if you want to go over what I changed, thanks.

brianpm commented 1 month ago

@justin-richling -- Let's discuss the changes. I think they are mostly fine.

There are two things that I'm not liking at first glance. First is the method load_obs_climo_dataset -- I don't see why we need to make "obs" a special method instead of only using "ref". If possible, I'd like to only ever deal with "ref" (and especially, I'd like script-writers to only have to worry about ref and never distinguish between obs and ref).

Second is related, and is the logic in load_da. My thinking is that load_da shouldn't care what kind of data is getting loaded. I'm thinking this logic should be pushed into the methods that call load_da. But it looks like you made the opposite determination and moved this logic into load_da... let's talk about it and decide how to deal with it.

justin-richling commented 1 month ago

@brianpm I've added the changes we discussed and cleaned up the methods. If you wouldn't mind giving adf_dataset.py another quick glance, we can probably pass this to @nusbaume for a final review. Thanks.

justin-richling commented 2 weeks ago

@nusbaume I think this is all good for your final review., let me know if you see anything that needs addressing.

justin-richling commented 2 weeks ago

@nusbaume I believe I addressed all the suggestions. If you wouldn't mind just giving it one last glance, I think it's on its way, thanks!