Skip to content

return eml_validate check in eml_otherEntity_to_dataTable#154

Merged
dmullen17 merged 3 commits intoNCEAS:masterfrom
dmullen17:master
Jan 14, 2020
Merged

return eml_validate check in eml_otherEntity_to_dataTable#154
dmullen17 merged 3 commits intoNCEAS:masterfrom
dmullen17:master

Conversation

@dmullen17
Copy link
Member

I found that this function doesn't return an error if the eml_validate check fails. Adding this in @jeanetteclark if you want to give it a quick review. Curious to see if any of the unit tests fail now as well.

@jeanetteclark
Copy link
Collaborator

Hey Dominic I need to look closer at the second commit you made but probably won't have time this week. Did you test the case where you already have a bunch of dataTables, and want to convert a single otherEntity to a dataTable? I'm also not really sure what lines 718-721 are doing - seems like duplicate entities from the editor should be handled elsewhere if that is what it is doing

@dmullen17
Copy link
Member Author

dmullen17 commented Dec 12, 2019

@jeanetteclark this what I mean by duplicate otherEntities created by the editor https://test.arcticdata.io/view/urn:uuid:d2710caa-9d77-4744-a76a-36a3e94705b7. I didn't phrase it well and if you can improve on that definitely update the comment because that code isn't self-explanatory.

Here's an example of the error I was getting. It wasn't showing up before because when validate_eml was set to TRUE then eml_validate would run, but it didn't actually cause an error, it just silently failed.

cn_staging <- CNode('STAGING')
adc_test <- getMNode(cn_staging,'urn:node:mnTestARCTIC')
doc <- read_eml(getObject(adc_test, 'urn:uuid:d2710caa-9d77-4744-a76a-36a3e94705b7'))
# the first one has attributes and the second doesn't - remove it
doc$dataset$otherEntity[[2]] <- NULL
# Pass doc into `eml_otherEntity_to_dataTable`

# Here's the first line that causes problems 
otherEntity <- doc$dataset$otherEntity
# If you print otherEntity to the console it's a list of length 1 (because it used to a list of length 2 but setting the 2nd otherEntity to NULL doesn't update it to the correct format)

# Ends up getting compounded by this line 
otherEntity <- list(otherEntity)
# Now if you print it you'll see [[1]][[1]] notation 

Copy link
Collaborator

@jeanetteclark jeanetteclark left a comment

Choose a reason for hiding this comment

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

Super minor change that can be committed directly - you were right @dmullen17 this was necessary. I think the comment in the code confused me about what was actually happening.

Co-Authored-By: Jeanette Clark <jclark@nceas.ucsb.edu>
@dmullen17 dmullen17 merged commit 2c371ce into NCEAS:master Jan 14, 2020
laijasmine pushed a commit that referenced this pull request Oct 2, 2020
return eml_validate check in eml_otherEntity_to_dataTable
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.

2 participants