JuliaMolSim / Molly.jl

Molecular simulation in Julia
Other
390 stars 53 forks source link

Fix fieldnames bug in AtomsBase interface #145

Closed ejmeitz closed 1 year ago

ejmeitz commented 1 year ago

Realized fieldnames was called on the system instance not the typeof the system which is not right.

ejmeitz commented 1 year ago

How do I change the SimpleCrystals compat to be just 0.1.5?? I changed stuff in 0.1.6 that broke a test here.

jgreener64 commented 1 year ago

= 0.1.5 but better to fix the test and up the compat to 0.1.6?

If you are following semver with that package you should also bump to 0.2.0 if there are breaking changes.

ejmeitz commented 1 year ago

Yeah I'll fix them, you mind if I just add that in this PR?

I didn't actually change the interface. The way I coded it in Molly just didn't use the interface (oops). I accessed a property directly (atom.sym) but I changed it to atomic_symbol in 0.1.6. I should just be calling the atomic_symbol function.

jgreener64 commented 1 year ago

Yes add it to this PR.

codecov[bot] commented 1 year ago

Codecov Report

Patch coverage: 75.00% and project coverage change: -0.06 :warning:

Comparison is base (58d3390) 73.37% compared to head (5a672de) 73.31%.

Additional details and impacted files ```diff @@ Coverage Diff @@ ## master #145 +/- ## ========================================== - Coverage 73.37% 73.31% -0.06% ========================================== Files 34 34 Lines 5022 5022 ========================================== - Hits 3685 3682 -3 - Misses 1337 1340 +3 ``` | [Impacted Files](https://app.codecov.io/gh/JuliaMolSim/Molly.jl/pull/145?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=JuliaMolSim) | Coverage Δ | | |---|---|---| | [src/types.jl](https://app.codecov.io/gh/JuliaMolSim/Molly.jl/pull/145?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=JuliaMolSim#diff-c3JjL3R5cGVzLmps) | `72.60% <75.00%> (ø)` | | ... and [1 file with indirect coverage changes](https://app.codecov.io/gh/JuliaMolSim/Molly.jl/pull/145/indirect-changes?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=JuliaMolSim)

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

ejmeitz commented 1 year ago

Ok changes made, no clue why the agent tests fail in one of the test sets and pass in the others.