Skip to content

refactor and add format plugin system#168

Merged
amcadmus merged 4 commits into
deepmodeling:masterfrom
njzjz:refactor
Jul 5, 2021
Merged

refactor and add format plugin system#168
amcadmus merged 4 commits into
deepmodeling:masterfrom
njzjz:refactor

Conversation

@njzjz

@njzjz njzjz commented Jul 3, 2021

Copy link
Copy Markdown
Member

fix #164.

@codecov-commenter

codecov-commenter commented Jul 3, 2021

Copy link
Copy Markdown

Codecov Report

Merging #168 (4ca3706) into master (5a93f08) will increase coverage by 0.48%.
The diff coverage is 82.79%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #168      +/-   ##
==========================================
+ Coverage   79.59%   80.07%   +0.48%     
==========================================
  Files          34       54      +20     
  Lines        4357     4553     +196     
==========================================
+ Hits         3468     3646     +178     
- Misses        889      907      +18     
Impacted Files Coverage Δ
dpdata/plugins/ase.py 19.23% <19.23%> (ø)
dpdata/plugins/pymatgen.py 25.80% <25.80%> (ø)
dpdata/plugins/rdkit.py 64.51% <64.51%> (ø)
dpdata/plugins/siesta.py 75.00% <75.00%> (ø)
dpdata/bond_order_system.py 61.29% <80.00%> (+0.04%) ⬆️
dpdata/plugins/list.py 81.25% <81.25%> (ø)
dpdata/plugins/amber.py 81.48% <81.48%> (ø)
dpdata/system.py 81.64% <83.54%> (+1.03%) ⬆️
dpdata/format.py 86.20% <86.20%> (ø)
dpdata/plugins/pwmat.py 86.48% <86.48%> (ø)
... and 35 more

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 5a93f08...4ca3706. Read the comment docs.

@njzjz njzjz mentioned this pull request Jul 3, 2021
Comment thread dpdata/plugins/gromacs.py Outdated

@Format.register("gro")
@Format.register("gromacs/gro")
class PwmatOutputFormat(Format):

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Incorrect class name. Shouldn't be something like GromacsGroFormat?

Comment thread dpdata/plugins/xyz.py Outdated

@Format.register("quip/gap/xyz")
@Format.register("quip/gap/xyz_file")
class DeePMDRawFormat(Format):

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Something like QuipGapXYZ?

Comment thread plugin_example/README.md Outdated
Comment on lines +14 to +22
> Data Summary
> Unlabeled System
> -------------------
> Frame Numbers : 12
> Atom Numbers : 20
> Element List :
> -------------------
> X
> 20 No newline at end of file

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Should be quoted with ```

Comment thread plugin_example/README.md
@@ -0,0 +1,22 @@
# An example for plugin

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

How about adding a reference in the main README to this example?

@amcadmus amcadmus requested a review from Ericwang6 July 4, 2021 09:20
Comment thread dpdata/plugins/rdkit.py


@Format.register("mol")
@Format.register("mol_file")

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Why here are 2 register decorators?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

All of the following usage will be supported:

  • from_mol(file_name)
  • from_mol_file(file_name)
  • to_mol(file_name)
  • to_mol_file(file_name)
  • BondOrderSystem(file_name, fmt='mol')
  • BondOrderSystem(file_name, fmt='mol_file')
  • to('mol', file_name)
  • to('mol_file', file_name)

@amcadmus amcadmus merged commit ea32d45 into deepmodeling:master Jul 5, 2021
@njzjz njzjz deleted the refactor branch July 5, 2021 14:26
@njzjz

njzjz commented Jul 5, 2021

Copy link
Copy Markdown
Member Author

It seems that I chose the wrong branch? We need to sync master with devel.

@amcadmus

amcadmus commented Jul 6, 2021

Copy link
Copy Markdown
Member

It seems that I chose the wrong branch? We need to sync master with devel.

Yes you are right...

njzjz added a commit to njzjz/dpdata that referenced this pull request Jul 13, 2021
amcadmus added a commit that referenced this pull request Jul 15, 2021
fix two bugs that implemented in #168
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.

[Feature Request] Format plugin system

4 participants