Skip to content

Fix up creation of variables with fill values#7

Merged
mhidas merged 5 commits intomasterfrom
fill_values
Oct 8, 2018
Merged

Fix up creation of variables with fill values#7
mhidas merged 5 commits intomasterfrom
fill_values

Conversation

@mhidas
Copy link
Copy Markdown
Contributor

@mhidas mhidas commented Oct 1, 2018

Fixes #5

Copy link
Copy Markdown
Contributor

@ggalibert ggalibert left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me, just a minor question.

Comment thread ncwriter/template.py
if 'data' not in var:
raise ValueError('No data specified for variable {varname}'.format(varname=varname))
if var['data'] is not None:
ncvar[:] = var['data']
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What happens to nc_var if var['data'] is None? I couldn't see where it might be set to all _FillValue.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fillvalue is setup at createVariable time, so if no data is added (this case, var['data'] is None), everything is fillvalue.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yep, that's the default behaviour. Unless you specify fill_value = False as an argument to createVariable(), you don't actually have to explicitly set the variable values to fill_value.

Copy link
Copy Markdown
Contributor

@ocehugo ocehugo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I just notice a typo in a new func

update_dimensinos to update_dimensions

Comment thread ncwriter/template.py
ncvar = self.ncobj.createVariable(varname, datatype)
else:
var_c_keys = list(self._create_var_opts(var))
var_attr = var.get('attributes', {})
Copy link
Copy Markdown
Contributor

@ocehugo ocehugo Oct 1, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

EDITED: I'm blind!

Why using get at all? Better to be explicit !?

Comment thread ncwriter/template.py
if 'data' not in var:
raise ValueError('No data specified for variable {varname}'.format(varname=varname))
if var['data'] is not None:
ncvar[:] = var['data']
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fillvalue is setup at createVariable time, so if no data is added (this case, var['data'] is None), everything is fillvalue.

@mhidas
Copy link
Copy Markdown
Contributor Author

mhidas commented Oct 8, 2018

Typo fixed and merge conflict resolved.

@mhidas mhidas merged commit a79b7ab into master Oct 8, 2018
@mhidas mhidas deleted the fill_values branch October 8, 2018 03:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants