search

ropensci / EML

Ecological Metadata Language interface for R: synthesis and integration of heterogenous data
https://docs.ropensci.org/EML
Other
98 stars 33 forks source link

make new missingValues feature smarter, improve docs, add tests #270

Closed jeanetteclark closed 5 years ago

jeanetteclark commented 5 years ago

Previous PR wasn't very smart about checking whether attributes had missing value codes in the attributes table, missing values table, or both. I improved this based on the bug @atn38 bought up in issue #268

I also improved the documentation for the function by listing what (I think...) are all of the supported column names in the attributes table.

I added an error check which prevents the function from failing silently if one of the attribute table's columns is not recognized by the set_attributes function due to typo or misremembering a column name.

I also added a few tests, which I should have done the first time around.

cboettig commented 5 years ago

@jeanetteclark nice, this looks good.

p.s. I think if you pull from the https://github.com/ropensci/EML master, you'll get a PR that doesn't include the previous commits(?) I think this is due to the fact that I've been using squash and merge to merge changes, which is rebasing the master branch.

jeanetteclark commented 5 years ago

yeah I'm not sure about this one - I did some reading and understand what is happening a bit better. (This SO post was helpful. I wonder if I should rebase my fork to the upstream as opposed to merging. Or I could start working on branches on my fork and send a PR from that

cboettig commented 5 years ago

right, I think you would also have to rebase, as git merge never wants to squash history, but it's not much of an issue anyway.