jamesrhester / CrystalInfoFramework.jl

Julia tools for reading Crystallographic Information Framework (CIF) files and dictionaries
GNU General Public License v3.0
12 stars 3 forks source link

`Cif` method that takes a file handle #10

Closed brainandforce closed 1 week ago

brainandforce commented 1 year ago

It appears that the only methods for reading in CIF file are

julia> methods(Cif)
# 4 methods for type constructor:
 [1] Cif()
     @ ~/.julia/packages/CrystalInfoFramework/lBBNH/src/cif_base.jl:313
 [2] Cif(s::AbstractString; verbose, version, source)
     @ ~/.julia/packages/CrystalInfoFramework/lBBNH/src/cif_base.jl:686
 [3] Cif(s::FilePathsBase.AbstractPath; verbose, native, version)
     @ ~/.julia/packages/CrystalInfoFramework/lBBNH/src/cif_base.jl:660
 [4] Cif(contents::Dict{String, T}, original_file::FilePathsBase.AbstractPath, header_comments::String) where {V, T<:CifContainer{V}}
     @ ~/.julia/packages/CrystalInfoFramework/lBBNH/src/cif_base.jl:304

Although it's not what I expected on my first attempt to use the package, I understand why a string argument is parsed as CIF contents instead of a file path - Julia really does need a dedicated path type. However, I would expect giving Cif() a file handle to work properly, so something along the lines of

open(io -> Cif(io), "this_is_a.cif")

could work.

jamesrhester commented 1 year ago

I still get caught trying to give a plain path to the Cif constructor and expecting it to open a file, and I wrote the thing! So perhaps the API should not interpret a string as a CIF file, but as a path.

Anyway, I like the suggestion to have an IO stream as an argument. Originally it was difficult to do this because the C cif_api library wanted to have a path string passed to it, and I couldn't get it to work with an already-open file handle. Now that there is a Julia native parser for Cifs, a method of the form Cif(io::IO) could be defined, so you could write Cif(open("my_cif_file.cif")). Is that what you were thinking of? If so, I (or you!) could add it pretty easily.

brainandforce commented 1 year ago

Yup, that's exactly what I was thinking. I'm not sure when I can get to this, but if I do I'll send in a PR.

jamesrhester commented 1 week ago

Version 0.7.0 (just released) reverts to interpreting a string passed to the Cif constructor as a file path.

jamesrhester commented 1 week ago

The latest commit (3fcfa27) now includes a CIF constructor with an IO first argument. Sorry for the delay.