[ghemical-devel] Re: OB-2.0 changes for ghemical

[Tommi Hassinen]

On Tue, 27 Sep 2005, Geoffrey Hutchison wrote:

> Hi Tommi,
>
> I've made some initial changes to integrate Open Babel 2.0 into ghemical (CVS 
> HEAD). Before I contribute any sort of patch, I'd like to ask a few 
> questions.
>
> * There are significant changes in how files are read under OB 2.0. Do you 
> want ghemical to support OB 1.x versions and try to detect them via 
> configure? Or should Ghemical 2.0 rely on OB 2.0? (Seems like a nice set of 
> version numbers. :-)

Hi,

I'm not really an expert myself what comes into configure scripts and such 
things. I think supporting both versions is an OK option, the hardest part 
is the configure script.

Now the configure script defines a macro ENABLE_OPENBABEL if we had 
--enable-openbabel at the command line. Perhaps this could be changed so 
that it would (additionally?) define either ENABLE_OPENBABEL_V1 or 
ENABLE_OPENBABEL_V2 (or whatever the macro names are) depending on what 
configuration was found on the system. The V1 option would be for the 
current version and V2 for the updated version.

> * Is the ghemical file format somewhat stable again? It seems like the 
> current CVS version will read OB output, and vice versa. Granted, the new 
> file format isn't compatible with OB 1.x code, but seems completely 
> compatible with OB-2.0.
>
> * Are there more changes needed to the OB-2.0 ghemical format support?

I might need to add a new !Section for saving secondary structure 
constraints and related information. But in OpenBabel I think this have to 
be handled so that these new and unknown keywords are just ignored and 
skipped.

So in this sense I expect the file format to be stable.

Please note that in ghemical-1.90 you might get a different file if you do 
a "save as" or an "export .gpr" operation. Also, at src/project.cpp 
starting from line 292 there is the function

bool project::ExportFile(const char * filename, int index)

where the file export is done ; there is a call

 	WriteGPR_v100((* this), intermed);

that should be changed into

 	WriteGPR((* this), intermed);

for most rigorous testing.

> Cheers,
> -Geoff
>
> P.S. On an unrelated note, do you want me to go and fix all the "backwards" 
> header warnings? I'm assuming that most people are using GCC-3.0 or an 
> otherwise recent ISO C++ compiler. It's pretty easy to run a sed script and 
> clean up the warnings.

I once started to convert these manually, but the job never finished... :(

It's OK if you fix the warnings. Also if you could you send me such a 
script, I would be glad to try it.

Regards,

 	Tommi