search

valeriupredoi / bgcval2

Package for BGCVal v2.0
3 stars 0 forks source link

Move `UKESMpython.py` module in `bgcvaltools` #84

Closed valeriupredoi closed 1 year ago

valeriupredoi commented 1 year ago

Closes #51

ledm commented 1 year ago

Not to throw a spanner in the works, but if we're changing it, perhaps UKESMpython is the wrong name for this tool? It contains a bunch of generic tools for bgcval. It would be more appropriately called bgcvaltool/bgcvaltools.py ? or is that confusing?

I'd also say maybe change import as ukp to import as bgcvt?

valeriupredoi commented 1 year ago

Not to throw a spanner in the works, but if we're changing it, perhaps UKESMpython is the wrong name for this tool? It contains a bunch of generic tools for bgcval. It would be more appropriately called bgcvaltool/bgcvaltools.py ? or is that confusing?

I'd also say maybe change import as ukp to import as bgcvt?

OK but don't name it bgcvaltools.py since that's the name of the dir where it lives - that's not good practice and will lead to module confusions. It's easy to do the renaming in this PR, just suggest some 30 odd code changes :grin:

ledm commented 1 year ago

okay, I'll think of another name. It's pretty easy to change something everywhere all at once using atom/notepad++ or visual studio. I'm happy to do it.

valeriupredoi commented 1 year ago

I'd suggest sed for that, but you gotta be careful not to change bits that are not meant to be changed :grin:

ledm commented 1 year ago

Okay. lets go with UKESMpython.py -> bv2tools.py and ukp -> bvt? (Why was this so hard to decide on a name?!)

valeriupredoi commented 1 year ago

Okay. lets go with UKESMpython.py -> bv2tools.py and ukp -> bvt? (Why was this so hard to decide on a name?!)

I like it! bvt sounds almost like BWT :grin: A522 P Side

valeriupredoi commented 1 year ago

just checked now - no more instance of UKESMpython nor ukp in both main dir and bgcval2/bgcval2 :+1:

ledm commented 1 year ago

The test is failing because the test is still running on debug.yml, instead of integration_test.yml.

valeriupredoi commented 1 year ago

cheers, bud! Merge at will pls :beer: I'm taking the rest of the day off - need a breather before ESMVal releases dance starts next week

ledm commented 1 year ago

Merged and thanks bud. Have a good weekend! May it be full of fun modelling, not work modelling.