Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[BUG] Fixed timestamps when there are more than one event #48

Merged
merged 3 commits into from
May 3, 2016

Conversation

stuwilkins
Copy link
Member

Attn @cmazzoli

@coveralls
Copy link

Coverage Status

Coverage remained the same at 27.303% when pulling 312171f on stuwilkins:fix_timestamps into 4a67124 on NSLS-II-CSX:master.

handler_overrides=hover)]
timestamps = img[0]['data']['fccd_image_lightfield']
timestamps = [i['data'][tag] for i in img if tag in i['data'].keys()]
Copy link
Member

Choose a reason for hiding this comment

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

This will silently drop images that do not have a time stamp which may make it difficult to realign the timestamps with the images later. Maybe something like

timestamps = [i['data'][tag] if tag in i['data'].keys() else np.nan for i in img]

which will fill in np.nan for the missing values. Or, as these are dictionaries coming back

timestamps = [d['data'].get(tag, np.nan) for d in img]

Copy link
Member Author

Choose a reason for hiding this comment

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

I see what you say, but this does not place np.nan for fccd timestamps that don't exist but rather for events that don't have a tag. Your suggestion would make the output list be of length of all events, not just events that have a tag. Perhaps both behaviors would be best to add with a fill kwarg

@codecov-io
Copy link

codecov-io commented Apr 26, 2016

Current coverage is 27.12%

Merging #48 into master will increase coverage by -0.17%

@@           master        #48   diff @@
========================================
  Files          11         11          
  Lines         304        306     +2   
  Methods         0          0          
  Branches        0          0          
========================================
  Hits           83         83          
- Misses        221        223     +2   
  Partials        0          0          

Powered by Codecov. Last updated by 4a67124...4029428

@coveralls
Copy link

Coverage Status

Coverage decreased (-0.2%) to 27.124% when pulling 61ed7f2 on stuwilkins:fix_timestamps into 4a67124 on NSLS-II-CSX:master.

@coveralls
Copy link

Coverage Status

Coverage remained the same at 27.303% when pulling dbef079 on stuwilkins:fix_timestamps into 4a67124 on NSLS-II-CSX:master.

@stuwilkins stuwilkins merged commit a6a0171 into NSLS-II-CSX:master May 3, 2016
stuwilkins pushed a commit that referenced this pull request May 3, 2016
Merge: 4a67124 dbef079
Author: Stuart Wilkins <stuwilkins@users.noreply.github.com>

    Merge pull request #48 from stuwilkins/fix_timestamps
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.

4 participants