search

SimonEnsemble / PorousMaterials.jl

Julia package towards classical molecular modeling of nanoporous materials
GNU General Public License v3.0
51 stars 11 forks source link

isotherm_sim_results_to_dataframe function #138

Closed ngantzler closed 4 years ago

ngantzler commented 4 years ago

I wrote a function isotherm_sim_results_to_dataframe() that takes the .jld2 output files from gcmc simulations and creates a DataFrame for specified properties.

Surluson commented 4 years ago

Nice addition :+1: I have a few comments that I think will have to be addressed before we merge:

1) I don't think this will work for every property in the .jld2 result file. The columns in the DataFrame are initialized as a Array{Float64, 1} but there are a few properties in the result files that are not Float64, such as repfactors, adsorbate, forcefield etc. I'm not sure what the best solution to this would be other than changing it to Array{Any, 1} or choose how to initialize based on the desired property.

2) The description for desired_props in the docstring is missing.

3) Tests need to be added

SimonEnsemble commented 4 years ago

@ngantzler can you remake this PR in the major_refactor branch? looks like this push is on the old master branch, not on major_refactor