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

Updated Readme, Docstrings datasets and small additions notebook #39

Merged
merged 7 commits into from
Mar 5, 2024

Conversation

S-Thaler
Copy link
Collaborator

@S-Thaler S-Thaler commented Mar 1, 2024

  • Updated dataset statistic (preliminary) in Readme.md
  • Added docstrings to datasetclasses, where missing (except DES and PubChemQC)
  • Added a little more explanation to the usage notebook
  • Added a few module and function docstrings

Additional fixes:

  • isort problems after deleting src dir
  • timeout problem when downloading large datasets

@S-Thaler S-Thaler requested a review from FNTwin March 1, 2024 23:27
@S-Thaler S-Thaler added the documentation Improvements or additions to documentation label Mar 1, 2024
@S-Thaler
Copy link
Collaborator Author

S-Thaler commented Mar 2, 2024

The import formatting in my pre-commit setup seems to be off, I'll need to fix that on Monday...

Copy link
Collaborator

@FNTwin FNTwin left a comment

Choose a reason for hiding this comment

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

Some changes or discussion required. Appreciated all the work.

openqdc/datasets/__init__.py Outdated Show resolved Hide resolved
openqdc/datasets/pcqm.py Outdated Show resolved Hide resolved
@FNTwin
Copy link
Collaborator

FNTwin commented Mar 2, 2024

The import formatting in my pre-commit setup seems to be off, I'll need to fix that on Monday...

It is probably because you added a docstring at the beginning of the files and most likely ruff or isort are complaining.
In any case i'll fix it in my branch so don't bother too much. Just check that you are running black>=24 and run:

ruff .
pre-commit run --all-files

Copy link
Collaborator Author

@S-Thaler S-Thaler left a comment

Choose a reason for hiding this comment

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

The import formatting in my pre-commit setup seems to be off, I'll need to fix that on Monday...

It is probably because you added a docstring at the beginning of the files and most likely ruff or isort are complaining.
In any case i'll fix it in my branch so don't bother too much. Just check that you are running black>=24 and run:

ruff .
pre-commit run --all-files

Seems to be some issue with isort. I'll leave it for now. pre-commit checks out locally for me.

Copy link
Collaborator

@FNTwin FNTwin left a comment

Choose a reason for hiding this comment

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

Thank you for the help Stephan!

@FNTwin FNTwin merged commit 9b70b84 into refactoring Mar 5, 2024
5 checks passed
@FNTwin FNTwin deleted the docstring_enhancements branch March 5, 2024 13:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation Improvements or additions to documentation
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants