Skip to content

Flexible subject reading - #73

Open
alessandrofelder wants to merge 2 commits into
masterfrom
flexible-subject-reading
Open

Flexible subject reading#73
alessandrofelder wants to merge 2 commits into
masterfrom
flexible-subject-reading

Conversation

@alessandrofelder

Copy link
Copy Markdown
Collaborator

This includes all non-empty information from the [BIOLOGY INFORMATION] section provided in headers of non-legacy LabView versions under the Subject group in the NWB file. If any of these header fields are non-empty, they will be written as a row in Subject, and any subject-related entries in the metadata will be ignored as a consequence.

Addresses one of the remaining points in #23 .

@ageorgou ageorgou left a comment

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.

The header method looks good, but I don't see where it's called or where self.subject is set.

return trial_times

def get_subject_info(self):
return "\n".join(

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.

I wonder whether ". ".join(... is better (to have everything in a single line. It could get quite long if we have a lot of information though.

Comment on lines +224 to +227
# If both metadata and header have a non-empty subject entry, the header gets priority.
# If neither have an entry, pynwb defaults to not writing a subject field in the file.
if self.subject:
self.add_subject(self.subject)

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.

Are we sure that this way prioritises the header over the metadata? If they both have subject information, does the second call to add_subject not do anything, or does it fail (or overwrite it with the second argument)?

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