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

[Feature] QM9Edge Dataset Support #2704

Merged
merged 17 commits into from
Mar 9, 2021
Merged

Conversation

hengruizhang98
Copy link
Contributor

@hengruizhang98 hengruizhang98 commented Feb 26, 2021

Description

QM9Edge Dataset Support. #2567
@mufeili

Checklist

Please feel free to remove inapplicable items for your PR.

  • The PR title starts with [$CATEGORY] (such as [NN], [Model], [Doc], [Feature]])
  • Changes are complete (i.e. I finished coding on this PR)
  • All changes have test coverage
  • Code is well-documented
  • To my best knowledge, examples are either not affected by this change,
    or have been fixed to be compatible with this change
  • Related issue is referred in this PR

@mufeili mufeili self-requested a review February 26, 2021 06:46
python/dgl/data/qm9_edge.py Outdated Show resolved Hide resolved
python/dgl/data/qm9_edge.py Outdated Show resolved Hide resolved
@mufeili
Copy link
Member

mufeili commented Mar 3, 2021

I still find this confusing. If a user has not installed rdkit, then this class will use the preprocessed dataset. Then when will a user ever want to preprocess from scratch and install rdkit for doing so? If you just want to display the details of preprocessing as @BarclayII suggested, then just always let users preprocess from scratch and save the preprocessed dataset to the local disk.

python/dgl/data/qm9_edge.py Outdated Show resolved Hide resolved
python/dgl/data/qm9_edge.py Outdated Show resolved Hide resolved
@mufeili
Copy link
Member

mufeili commented Mar 3, 2021

You also need to update this file to update the documentation.

python/dgl/data/qm9_edge.py Outdated Show resolved Hide resolved
python/dgl/data/qm9_edge.py Outdated Show resolved Hide resolved
python/dgl/data/qm9_edge.py Outdated Show resolved Hide resolved
python/dgl/data/qm9_edge.py Outdated Show resolved Hide resolved
python/dgl/data/qm9_edge.py Outdated Show resolved Hide resolved
python/dgl/data/qm9_edge.py Outdated Show resolved Hide resolved
python/dgl/data/qm9_edge.py Outdated Show resolved Hide resolved
python/dgl/data/qm9_edge.py Outdated Show resolved Hide resolved
python/dgl/data/qm9_edge.py Outdated Show resolved Hide resolved
python/dgl/data/qm9_edge.py Outdated Show resolved Hide resolved
python/dgl/data/qm9_edge.py Outdated Show resolved Hide resolved
python/dgl/data/qm9_edge.py Outdated Show resolved Hide resolved
python/dgl/data/qm9_edge.py Outdated Show resolved Hide resolved
python/dgl/data/qm9_edge.py Outdated Show resolved Hide resolved
python/dgl/data/qm9_edge.py Outdated Show resolved Hide resolved
python/dgl/data/qm9_edge.py Outdated Show resolved Hide resolved
python/dgl/data/qm9_edge.py Outdated Show resolved Hide resolved
python/dgl/data/qm9_edge.py Outdated Show resolved Hide resolved
python/dgl/data/qm9_edge.py Outdated Show resolved Hide resolved
python/dgl/data/qm9_edge.py Outdated Show resolved Hide resolved
python/dgl/data/qm9_edge.py Outdated Show resolved Hide resolved
python/dgl/data/qm9_edge.py Outdated Show resolved Hide resolved
python/dgl/data/qm9_edge.py Outdated Show resolved Hide resolved
@mufeili
Copy link
Member

mufeili commented Mar 5, 2021

I talked to Minjie. He suggested moving the code for preprocessing to a gist file and leave a link to the gist file.

@hengruizhang98
Copy link
Contributor Author

I talked to Minjie. He suggested moving the code for preprocessing to a gist file and leave a link to the gist file.

Now the preprocessing part has been moved to : https://gist.github.com/hengruizhang98/a2da30213b2356fff18b25385c9d3cd2

python/dgl/data/qm9.py Outdated Show resolved Hide resolved
python/dgl/data/qm9.py Outdated Show resolved Hide resolved
python/dgl/data/qm9.py Outdated Show resolved Hide resolved
python/dgl/data/qm9.py Outdated Show resolved Hide resolved
python/dgl/data/qm9.py Outdated Show resolved Hide resolved
python/dgl/data/qm9.py Outdated Show resolved Hide resolved
Copy link
Member

@mufeili mufeili left a comment

Choose a reason for hiding this comment

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

LGTM.

@mufeili mufeili merged commit a1f59c3 into dmlc:master Mar 9, 2021
@hengruizhang98 hengruizhang98 deleted the qm9_edge branch March 13, 2021 12:14
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