Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Cif method that takes a file handle #10

Closed
brainandforce opened this issue Jun 28, 2023 · 4 comments
Closed

Cif method that takes a file handle #10

brainandforce opened this issue Jun 28, 2023 · 4 comments

Comments

@brainandforce
Copy link

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
Copy link
Owner

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
Copy link
Author

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
Copy link
Owner

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

@jamesrhester
Copy link
Owner

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants