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.

Sign up for GitHub

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

Jump to bottom

Propagate changes from libsonata ElementReport #62

Merged
merged 10 commits into from
Jul 23, 2020
Merged

Propagate changes from libsonata ElementReport #62

merged 10 commits into from
Jul 23, 2020

Conversation

tomdele
Copy link
Contributor

@tomdele tomdele commented Jun 12, 2020

preparation for the new libsonata reports

@tomdele
Copy link
Contributor Author

tomdele commented Jun 16, 2020

This prepares the bluepysnap for the libsonata bump.
All tests are passing locally with the BlueBrain/libsonata#93

@tomdele tomdele changed the title Propagate changes in libsonata ElementReport Propagate changes froml libsonata ElementReport Jun 16, 2020
@tomdele tomdele changed the title Propagate changes froml libsonata ElementReport Propagate changes from libsonata ElementReport Jun 16, 2020
@tomdele tomdele force-pushed the new_libsonata_reports branch from 0c71d69 to 0dd7b00 Compare July 7, 2020 12:58
@tomdele
Copy link
Contributor Author

tomdele commented Jul 7, 2020 •
edited
Loading

If I am lucky enough the changes for the numpy array will be handled seamlessly with these changes ....

@tomdele tomdele requested a review from asanin-epfl July 7, 2020 15:40
@tomdele
Copy link
Contributor Author

tomdele commented Jul 7, 2020 •
edited
Loading

@asanin-epfl I will need your review when this is finished.
This is to handle the new version of the libsonata report outputs. I will ping you when I need your review on this. I don t think I will do drastic changes so you can already have a quick look.

@tomdele
Copy link
Contributor Author

tomdele commented Jul 7, 2020

The CI cannot pass due to an unreleased version of libsonata (0.1.4)
BlueBrain/libsonata#96

@tomdele
Copy link
Contributor Author

tomdele commented Jul 8, 2020

Need to remove the fix_libsonata when this change is pulled out from libsonata:
https://github.com/BlueBrain/snap/pull/62/files#diff-67a29d152e00a31a251cd47da15b2e55R300

@asanin-epfl
Copy link
Contributor

lgtm to me.

@tomdele
Copy link
Contributor Author

tomdele commented Jul 8, 2020

Thx @asanin-epfl !
I will ping you again when all the changes will be done and when I will create a real PR.

@tomdele tomdele force-pushed the new_libsonata_reports branch from 921233e to 12c6077 Compare July 14, 2020 12:17
@tomdele tomdele force-pushed the new_libsonata_reports branch from 7235fee to 7e5f2d2 Compare July 22, 2020 13:17
@tomdele
Copy link
Contributor Author

tomdele commented Jul 23, 2020

retest this please

@tomdele tomdele force-pushed the new_libsonata_reports branch from 7e5f2d2 to c8726fd Compare July 23, 2020 09:33
@codecov-commenter
Copy link

codecov-commenter commented Jul 23, 2020 •
edited
Loading

Codecov Report

Merging #62 into master will not change coverage.
The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff            @@
##            master       #62   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files           18        18           
  Lines         1342      1340    -2     
=========================================
- Hits          1342      1340    -2
Impacted Files Coverage Δ
bluepysnap/_plotting.py 100.00% <ø> (ø)
bluepysnap/nodes.py 100.00% <ø> (ø)
bluepysnap/utils.py 100.00% <ø> (ø)
bluepysnap/frame_report.py 100.00% <100.00%> (ø)
bluepysnap/spike_report.py 100.00% <100.00%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 639ac58...bea0529. Read the comment docs.

@tomdele tomdele marked this pull request as ready for review July 23, 2020 11:24
@tomdele tomdele mentioned this pull request Jul 23, 2020
Merged
@tomdele
Copy link
Contributor Author

tomdele commented Jul 23, 2020

@asanin-epfl you can review.

@asanin-epfl
Copy link
Contributor

ok, starting

@tomdele
Copy link
Contributor Author

tomdele commented Jul 23, 2020

Thanks a lot.

asanin-epfl
asanin-epfl approved these changes Jul 23, 2020
View reviewed changes
Copy link
Contributor

@asanin-epfl asanin-epfl left a comment

Choose a reason for hiding this comment

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

lgtm

bluepysnap/frame_report.py

res = pd.DataFrame(data=view.data,
columns=pd.MultiIndex.from_arrays(np.asarray(view.ids).T),
index=view.times).sort_index(axis=1)
Copy link
Contributor

Choose a reason for hiding this comment

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

view.times are always guaranteed to be present?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yes !
the view always contains the 3 objects

@tomdele tomdele merged commit 1619edd into master Jul 23, 2020
@tomdele
Copy link
Contributor Author

tomdele commented Jul 23, 2020

thank you for the review @asanin-epfl !

@tomdele tomdele deleted the new_libsonata_reports branch July 23, 2020 12:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@asanin-epfl asanin-epfl asanin-epfl approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@tomdele @asanin-epfl @codecov-commenter