Conversation
|
boss level, @davidhassell 🍺 Did you want to add a test that runs the demo or some unit/integration tests? No probs if not, we can do that later 👍 |
|
Cheers, @valeriupredoi. Don't mind either way on the tests question. |
| self.ncvar = ncvar | ||
|
|
||
| nc = netCDF4.Dataset(self.filename, "r") | ||
| v = nc.variables[self.ncvar] |
There was a problem hiding this comment.
What happens if the keyword ncvar is not present? Would it be better to pass a NetCDF dataset (already instantiated) in along with a variable name (both required)?
There was a problem hiding this comment.
(There would be some consequences. I would work through them, but you will have seen I have a git problem right now.)
There was a problem hiding this comment.
You'll get an exception with no ncvar or no filename. I don't think that it makes sense to pass in a netCDF.Dataset instance, as on principle we don't want those hanging around as open file handles, but we could make the __init__ args positional only. I'll change that.
bnlawrence
left a comment
There was a problem hiding this comment.
I'm happy with this. @valeriupredoi Can you please merge.
|
cheers, gents! 🍺 |
Hi, not much to say here, other than I have address the commenting points discussed on Thursday. Also
cfdmis no longer a dependency, as I now instantiate things indemo.pyas