pymatgen icon indicating copy to clipboard operation
pymatgen copied to clipboard

WIP: Adding support for reading vaspwave.h5 in Wavecar and Chgcar class

Open Shibu778 opened this issue 10 months ago • 4 comments

Dear Maintainers, This is my first contribution to pymatgen. I am relatively new to opensource and would appreciate your feedback and guidance in improving the added features and this PR. In further commits, I will add read_from_hdf5 functionality to Chgcar and write all the tests.

Summary

Major changes:

  • feature 1: Added a method read_from_hdf5 in Wavecar class to read WAVECAR related information from vaspwave.h5 file (refer to vaspwiki).
  • fix 1: made a function to read the legacy WAVECAR format that was previously implemented in Wavecar class.
  • fix 2: added appropriate comments and doc-string
  • fix 3: Some part of the pymatgen/io/vasp/outputs.py file got formatted by black formatter

Todos

This is work in progress, following needs to be done

  • feature 2: Add the same feature in Chgcar class
  • fix 4: Write appropriate test to read the hdf5 files in using Wavecar and Chgcar class

Pre-commit result

pre-commit run --all-files passed all everything

~\Github\pymatgen> pre-commit run --all-files
ruff.....................................................................Passed
ruff-format..............................................................Passed
check yaml...............................................................Passed
fix end of files.........................................................Passed
trim trailing whitespace.................................................Passed
mypy.....................................................................Passed
codespell................................................................Passed
cython-lint..............................................................Passed
double-quote Cython strings..............................................Passed
blacken-docs.............................................................Passed
markdownlint.............................................................Passed
nbstripout...............................................................Passed

Checklist

  • [x] Google format doc strings added. Check with ruff.
  • [x] Type annotations included. Check with mypy.
  • [x] The tests for fixes in Wavecar class remains unchanged to read legacy WAVECAR file
  • [x] Tests added for new features.

Summary by CodeRabbit

  • Refactor

    • Improved code readability and formatting in various functions.
    • Updated function signatures for clarity.
  • Documentation

    • Enhanced function descriptions with new comments and docstrings.

Shibu778 avatar Apr 18 '24 10:04 Shibu778

Walkthrough

The recent changes across the pymatgen/io/vasp/outputs.py and tests/io/vasp/test_outputs.py files focus on improving code readability and maintainability. These modifications include importing h5py, enhancing tuple unpacking, refining code formatting, updating function signatures with type annotations, and enriching documentation through comments and docstrings.

Changes

File Path Changes Summary
pymatgen/io/vasp/outputs.py Added h5py import, refactored tuple unpacking, improved formatting, updated function signatures, and enriched documentation.
tests/io/vasp/test_outputs.py Restructured code for readability and maintainability, improved code organization without altering core functionality.

🐰✨ A hop of joy for cleaner code,
With tuples unpacked along the road.
Type hints and docs, now clear as day,
In outputs.py, improvements stay.
Cheers to the devs, with ears so long,
For making the code both swift and strong.
🌟📄


Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media?

Share
Tips

Chat

There are 3 ways to chat with CodeRabbit:

  • Review comments: Directly reply to a review comment made by CodeRabbit. Example:
    • I pushed a fix in commit <commit_id>.
    • Generate unit testing code for this file.
    • Open a follow-up GitHub issue for this discussion.
  • Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query. Examples:
    • @coderabbitai generate unit testing code for this file.
    • @coderabbitai modularize this function.
  • PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
    • @coderabbitai generate interesting stats about this repository and render them as a table.
    • @coderabbitai show all the console.log statements in this repository.
    • @coderabbitai read src/utils.ts and generate unit testing code.
    • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.

Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments.

CodeRabbit Commands (invoked as PR comments)

  • @coderabbitai pause to pause the reviews on a PR.
  • @coderabbitai resume to resume the paused reviews.
  • @coderabbitai review to trigger a review. This is useful when automatic reviews are disabled for the repository.
  • @coderabbitai resolve resolve all the CodeRabbit review comments.
  • @coderabbitai help to get help.

Additionally, you can add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.

CodeRabbit Configration File (.coderabbit.yaml)

  • You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
  • Please see the configuration documentation for more information.
  • If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json

Documentation and Community

  • Visit our Documentation for detailed information on how to use CodeRabbit.
  • Join our Discord Community to get help, request features, and share feedback.
  • Follow us on X/Twitter for updates and announcements.

coderabbitai[bot] avatar Apr 18 '24 10:04 coderabbitai[bot]

Thanks for the PR @Shibu778! My sincere apologies there was no attention given to this PR before now.

Could you resolve the current merge conflicts and ensure the automated CI checks still pass? The pymatgen code has now been moved into /src directory, so the changes may just need to be moved.

Other than this, I notice a lot of code is duplicated between read_from_hdf5 and read_from_legacy. I wonder if maybe the duplicated code can be merged into a single function?

It should also be possible to detect the filetype automatically: use the existing logical if the filename is *WAVECAR*, and use the new logic if the filename is *h5*.

mkhorton avatar Jul 16 '24 22:07 mkhorton

Dear @janosh Thanks for your suggestion. Currently, I am trying to fix the merge conflict. I tried different approaches, and it's becoming complicated day by day. I am 8 commit ahead of the original repository and 304 commits behind. My effort in resolving the conflict first is polluting this pull request. What do you suggest I should do? Should we close this pull request and start a new one?

Shibu778 avatar Oct 05 '24 12:10 Shibu778

Now I have synced the forked repository properly. I will start adding the feature reading WAVECAR and CHGCAR in hdf5 format soon.

Shibu778 avatar Oct 05 '24 12:10 Shibu778