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

Add class-resolver for Aggregation #4716

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

cthoyt
Copy link
Contributor

@cthoyt cthoyt commented May 25, 2022

As a follow-up to #4687, this pull request adds a class-resolver for aggregations in torch_geometric.nn.aggr. It also adds a class docstring with an example on how to use it.

Note that class_resolver is a pure python package with no dependencies.

@codecov
Copy link

codecov bot commented May 25, 2022

Codecov Report

Merging #4716 (1cb1f74) into master (cb92831) will increase coverage by 0.00%.
The diff coverage is 100.00%.

@@           Coverage Diff           @@
##           master    #4716   +/-   ##
=======================================
  Coverage   82.40%   82.40%           
=======================================
  Files         321      321           
  Lines       17207    17209    +2     
=======================================
+ Hits        14179    14181    +2     
  Misses       3028     3028           
Impacted Files Coverage Δ
torch_geometric/nn/aggr/__init__.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 cb92831...1cb1f74. Read the comment docs.

@cthoyt
Copy link
Contributor Author

cthoyt commented May 25, 2022

schönes Feierabend btw

Copy link
Member

@rusty1s rusty1s left a comment

Choose a reason for hiding this comment

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

Thanks for the PR :)

@@ -11,6 +11,7 @@
'requests',
'pyparsing',
'scikit-learn',
'class_resolver',
Copy link
Member

Choose a reason for hiding this comment

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

We cannot add class-resolver package back in since it is not available on anaconda.org. That's why we started writing our own solution for that (with a limited feature set): https://github.com/pyg-team/pytorch_geometric/blob/master/torch_geometric/nn/resolver.py

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Hmm I can look into putting it in conda

@rusty1s rusty1s changed the title Add class-resolver for aggregations Add class-resolver for Aggregation May 26, 2022
@rusty1s rusty1s mentioned this pull request Jun 7, 2022
26 tasks
@rusty1s
Copy link
Member

rusty1s commented Jun 7, 2022

@cthoyt We added basic support for aggregation_resolver in #4749. Let me know once class-resolver is available on conda and I am happy to make the switch.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants