search

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

Warning about method overwrites #12

Closed kbarros closed 1 week ago

kbarros commented 3 weeks ago

This package seems to require FilePathsBase to load files. Precompiling the two packages together seems to give the following warnings:

WARNING: Method definition (::Type{URIs.URI})(FilePathsBase.AbstractPath) in module FilePaths at /home/runner/.julia/packages/FilePaths/ULnpu/src/uris.jl:5 overwritten in module CrystalInfoFramework at /home/runner/.julia/packages/CrystalInfoFramework/FpKpL/src/ddlm_dictionary_ng.jl:1304.
WARNING: Method definition kwcall(NamedTuple{names, T} where T<:Tuple where names, Type{URIs.URI}, FilePathsBase.AbstractPath) in module FilePaths at /home/runner/.julia/packages/FilePaths/ULnpu/src/uris.jl:5 overwritten in module CrystalInfoFramework at /home/runner/.julia/packages/CrystalInfoFramework/FpKpL/src/ddlm_dictionary_ng.jl:1304.

See, for example, the output of julia-runtest here: https://github.com/SunnySuite/Sunny.jl/actions/runs/11115137251/job/30882999280

kbarros commented 3 weeks ago

Maybe the ideal solution is to remove the FilePaths dependency, and expect the file path as a string. This would be more Julian, and aligns with previous comment here: https://github.com/jamesrhester/CrystalInfoFramework.jl/issues/10#issuecomment-1614359394

jamesrhester commented 3 weeks ago

Hmm, looks like I've indulged in some type piracy. I agree that it is time to revert to intepreting strings in the constructor as file paths rather than a piece of CIF.

jamesrhester commented 1 week ago

Just-released v0.7.0 removes FilePaths. It will still accept Path objects in the Cif constructor, but now an AbstractString is interpreted as a filename instead of CIF-formatted content.