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

AbstractGraphReporter.network_measures should not be abstract #281

Merged
merged 1 commit into from
Sep 21, 2020

Conversation

jayqi
Copy link
Collaborator

@jayqi jayqi commented Sep 3, 2020

AbstractGraphReporter.network_measures should not be abstract. The change in this PR implements the same logic as in the R package. For now though, trying to call it will result in pkg_graph.graph_measures() raising a NotImplementedError, which is sensible.

Fixes the issue reported here: #267 (comment)

@jayqi jayqi requested a review from a team September 3, 2020 04:05
@jayqi jayqi mentioned this pull request Sep 3, 2020
5 tasks
Copy link
Collaborator

@jameslamb jameslamb left a comment

Choose a reason for hiding this comment

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

makes sense to me!

@bburns632
Copy link
Collaborator

LGTM. There's a failed test due to gnutls loading failure in Travis. I'd like to look into that, but if we're still looking into that by this time tomorrow, it's probably safe to merge.

@jayqi
Copy link
Collaborator Author

jayqi commented Sep 3, 2020

It might be that the Mac build (with the gnutls error) was canceled because the Linux build had a C++ compilation error when installing devtools. The builds don't really matter at this point because the py-pkg branch doesn't touch the R code, and I don't have tests for the Python package yet.

@jameslamb
Copy link
Collaborator

It might be that the Mac build (with the gnutls error) was canceled because the Linux build had a C++ compilation error when installing devtools. The builds don't really matter at this point because the py-pkg branch doesn't touch the R code, and I don't have tests for the Python package yet.

@jayqi I'm ok with this, you can use your admin rights to merge this one

@bburns632 bburns632 merged commit fdf5444 into uptake:py-pkg Sep 21, 2020
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.

3 participants