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
Remove deprecations included in 0.10 release #969
Conversation
Code Climate has analyzed commit 96d6bcb and detected 1 issue on this pull request. Here's the issue category breakdown:
View more on Code Climate. |
Codecov Report
@@ Coverage Diff @@
## develop #969 +/- ##
=========================================
- Coverage 84.4% 84.4% -<.1%
=========================================
Files 58 53 -5
Lines 4945 5168 +223
=========================================
+ Hits 4176 4364 +188
- Misses 769 804 +35
Continue to review full report at Codecov.
|
Deprecated, use :meth:`from_networkx`. | ||
edge_type_name: | ||
Deprecated, use :meth:`from_networkx`. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There's a couple of deprecated arguments still here. (+ graph
and node_type_name
above that github wont let me suggest changes for)
Deprecated, use :meth:`from_networkx`. | |
edge_type_name: | |
Deprecated, use :meth:`from_networkx`. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I had carefully left these ones here despite removing the adjacent edge_weight_label
because these arguments are not properly deprecated: they don't trigger a runtime warning yet. I think it's important that we use a runtime warning to prompt people to move because:
- user with existing code won't re-read the docs, and
- they may not realise that a breaking change listed in the CHANGELOG affects their code (as in, they may not remember all the details about their code)
These arguments don't trigger a runtime warning yet because "FIXME(#717): this should have a deprecation warning, once the tests and examples have stopped using it" on line 253/241 below.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
cool that makes sense; everything looks good then
node_features: | ||
Deprecated, use :meth:`from_networkx`. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
node_features: | |
Deprecated, use :meth:`from_networkx`. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍
This includes simplifying
BlogCatalog3.load
because we no longer need to supportload_dataset_BlogCatalog3
.See: #959