brainandforce / Electrum.jl

A Julian toolkit for solid-state chemical theory.
MIT License
31 stars 0 forks source link

Helping abinit file reading/writing functions conform with standards #145

Closed brainandforce closed 1 year ago

brainandforce commented 1 year ago

I noticed that some of the methods for abinit file writing didn't conform to the (informal, undocumented) standard for file processing methods. Along with having the methods comply with that standard, I refactored them slightly and included some comments for improvement. I realize I should have been more proactive with that feedback earlier on. Since I don't work with anaddb I'd like to get your feedback before merging.

As a future note, I'm going to include some extra documentation for how to contribute file writing methods. The tl;dr is that every file writing function should have a method that accepts a file handle (Julia IO subtypes, see the method signatures that contain (io::IO, args...; kwargs...)) and then another method that accepts a path. By default we assumed that this was an AbstractString, but some packages provide a custom type for paths, so in the future there will be no type restriction.

codecov[bot] commented 1 year ago

Codecov Report

Patch coverage has no change and project coverage change: +0.07 :tada:

Comparison is base (d3bdfe9) 51.68% compared to head (c5a447b) 51.76%.

:exclamation: Current head c5a447b differs from pull request most recent head 341a967. Consider uploading reports for the commit 341a967 to get more accurate results

Additional details and impacted files ```diff @@ Coverage Diff @@ ## main #145 +/- ## ========================================== + Coverage 51.68% 51.76% +0.07% ========================================== Files 17 18 +1 Lines 1302 1300 -2 ========================================== Hits 673 673 + Misses 629 627 -2 ``` | [Impacted Files](https://app.codecov.io/gh/brainandforce/Electrum.jl/pull/145?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Brandon+Flores) | Coverage Δ | | |---|---|---| | [src/Electrum.jl](https://app.codecov.io/gh/brainandforce/Electrum.jl/pull/145?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Brandon+Flores#diff-c3JjL0VsZWN0cnVtLmps) | `33.33% <ø> (ø)` | | | [src/data/bands.jl](https://app.codecov.io/gh/brainandforce/Electrum.jl/pull/145?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Brandon+Flores#diff-c3JjL2RhdGEvYmFuZHMuamw=) | `0.00% <0.00%> (ø)` | | | [src/data/kpoints.jl](https://app.codecov.io/gh/brainandforce/Electrum.jl/pull/145?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Brandon+Flores#diff-c3JjL2RhdGEva3BvaW50cy5qbA==) | `71.42% <0.00%> (ø)` | | | [src/software/abinit.jl](https://app.codecov.io/gh/brainandforce/Electrum.jl/pull/145?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Brandon+Flores#diff-c3JjL3NvZnR3YXJlL2FiaW5pdC5qbA==) | `46.83% <0.00%> (+0.44%)` | :arrow_up: |

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Do you have feedback about the report comment? Let us know in this issue.

xamberl commented 1 year ago

Thanks for the comments. I do want to say that I noted differences in the header information over different versions of ABINIT and anaddb. The this function will need to be rewritten later to accommodate this.

brainandforce commented 1 year ago

I'm going to push this change to main since the functions are not exported and they'll need to be rewritten anyway.