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

Fix issue #299 #300

Merged
merged 2 commits into from
Jun 17, 2024
Merged

Fix issue #299 #300

merged 2 commits into from
Jun 17, 2024

Conversation

lohedges
Copy link
Contributor

This PR closes #299 but not using sire.legacy.Base.wrap for string values in the property_map, which were incorrectly being cast to GeneralUnit. This allows Mol2 files to be written from by BioSimSpace.IO.saveMolecules. This superseds #298.

  • I confirm that I have merged the latest version of devel into this branch before issuing this pull request (e.g. by running git pull origin devel): [y]
  • I confirm that I have permission to release this code under the GPL3 license: [y]

@lohedges lohedges added the bug Something isn't working label Jun 12, 2024
@lohedges lohedges requested a review from mb2055 June 12, 2024 15:44
@lohedges
Copy link
Contributor Author

Strange, I confirmed that all tests passed locally before submitting but there are failures on all platforms. Looks like an issue with RDKit conversion, which I don't think is related to this. Will debug tomorrow.

@lohedges
Copy link
Contributor Author

It looks like the RDKit pin might have failed and it's pulling in a version that is incompatible with the one that Sire was built against.

@lohedges
Copy link
Contributor Author

Okay, all of the failed CI runs pulled in RDKit version 2024.03.3. The last Sire devel build used 2024.03.2. Surely there can't be a breaking API change with a point release? In the Sire recipe we pin RDKit to the minor version number, i.e.:

  run_constrained:
    - {{ pin_compatible('rdkit', max_pin='x.x') }}

I'll try reproducing the environment locally to see if I can figure out what's wrong.

@lohedges
Copy link
Contributor Author

It seems that the SONAMEs are minor version specific, so we'll need to update the pin to 'x.x.x'.

ls -l 2024.03.2/lib | head -n 20
total 30080
drwxr-xr-x 3 lester lester    4096 Jun 13 09:36 cmake
drwxr-xr-x 3 lester lester    4096 Jun 13 09:36 python3.11
lrwxrwxrwx 1 lester lester      36 May  2 17:10 libRDKitAbbreviations.so -> libRDKitAbbreviations.so.1.2024.03.2
lrwxrwxrwx 1 lester lester      36 May  2 17:10 libRDKitAbbreviations.so.1 -> libRDKitAbbreviations.so.1.2024.03.2
-rwxr-xr-x 1 lester lester  176384 May  2 17:10 libRDKitAbbreviations.so.1.2024.03.2
lrwxrwxrwx 1 lester lester      32 May  2 17:10 libRDKitAlignment.so -> libRDKitAlignment.so.1.2024.03.2
lrwxrwxrwx 1 lester lester      32 May  2 17:10 libRDKitAlignment.so.1 -> libRDKitAlignment.so.1.2024.03.2
-rwxr-xr-x 1 lester lester   46376 May  2 17:10 libRDKitAlignment.so.1.2024.03.2
lrwxrwxrwx 1 lester lester      34 May  2 17:10 libRDKitavalon_clib.so -> libRDKitavalon_clib.so.1.2024.03.2
lrwxrwxrwx 1 lester lester      34 May  2 17:10 libRDKitavalon_clib.so.1 -> libRDKitavalon_clib.so.1.2024.03.2
-rwxr-xr-x 1 lester lester  517632 May  2 17:10 libRDKitavalon_clib.so.1.2024.03.2
lrwxrwxrwx 1 lester lester      32 May  2 17:10 libRDKitAvalonLib.so -> libRDKitAvalonLib.so.1.2024.03.2
lrwxrwxrwx 1 lester lester      32 May  2 17:10 libRDKitAvalonLib.so.1 -> libRDKitAvalonLib.so.1.2024.03.2
-rwxr-xr-x 1 lester lester   84336 May  2 17:10 libRDKitAvalonLib.so.1.2024.03.2
lrwxrwxrwx 1 lester lester      31 May  2 17:10 libRDKitCatalogs.so -> libRDKitCatalogs.so.1.2024.03.2
lrwxrwxrwx 1 lester lester      31 May  2 17:10 libRDKitCatalogs.so.1 -> libRDKitCatalogs.so.1.2024.03.2
-rwxr-xr-x 1 lester lester   16416 May  2 17:10 libRDKitCatalogs.so.1.2024.03.2
lrwxrwxrwx 1 lester lester      39 May  2 17:10 libRDKitChemicalFeatures.so -> libRDKitChemicalFeatures.so.1.2024.03.2
lrwxrwxrwx 1 lester lester      39 May  2 17:10 libRDKitChemicalFeatures.so.1 -> libRDKitChemicalFeatures.so.1.2024.03.2

ls -l 2024.03.3/lib | head -n 20
total 30100
drwxr-xr-x 3 lester lester    4096 Jun 13 09:36 cmake
drwxr-xr-x 3 lester lester    4096 Jun 13 09:36 python3.11
lrwxrwxrwx 1 lester lester      36 Jun  4 10:34 libRDKitAbbreviations.so -> libRDKitAbbreviations.so.1.2024.03.3
lrwxrwxrwx 1 lester lester      36 Jun  4 10:33 libRDKitAbbreviations.so.1 -> libRDKitAbbreviations.so.1.2024.03.3
-rwxr-xr-x 1 lester lester  176384 Jun  4 10:34 libRDKitAbbreviations.so.1.2024.03.3
lrwxrwxrwx 1 lester lester      32 Jun  4 10:34 libRDKitAlignment.so -> libRDKitAlignment.so.1.2024.03.3
lrwxrwxrwx 1 lester lester      32 Jun  4 10:33 libRDKitAlignment.so.1 -> libRDKitAlignment.so.1.2024.03.3
-rwxr-xr-x 1 lester lester   46376 Jun  4 10:34 libRDKitAlignment.so.1.2024.03.3
lrwxrwxrwx 1 lester lester      34 Jun  4 10:34 libRDKitavalon_clib.so -> libRDKitavalon_clib.so.1.2024.03.3
lrwxrwxrwx 1 lester lester      34 Jun  4 10:33 libRDKitavalon_clib.so.1 -> libRDKitavalon_clib.so.1.2024.03.3
-rwxr-xr-x 1 lester lester  517632 Jun  4 10:34 libRDKitavalon_clib.so.1.2024.03.3
lrwxrwxrwx 1 lester lester      32 Jun  4 10:34 libRDKitAvalonLib.so -> libRDKitAvalonLib.so.1.2024.03.3
lrwxrwxrwx 1 lester lester      32 Jun  4 10:33 libRDKitAvalonLib.so.1 -> libRDKitAvalonLib.so.1.2024.03.3
-rwxr-xr-x 1 lester lester   84336 Jun  4 10:34 libRDKitAvalonLib.so.1.2024.03.3
lrwxrwxrwx 1 lester lester      31 Jun  4 10:34 libRDKitCatalogs.so -> libRDKitCatalogs.so.1.2024.03.3
lrwxrwxrwx 1 lester lester      31 Jun  4 10:33 libRDKitCatalogs.so.1 -> libRDKitCatalogs.so.1.2024.03.3
-rwxr-xr-x 1 lester lester   16416 Jun  4 10:34 libRDKitCatalogs.so.1.2024.03.3
lrwxrwxrwx 1 lester lester      39 Jun  4 10:34 libRDKitChemicalFeatures.so -> libRDKitChemicalFeatures.so.1.2024.03.3
lrwxrwxrwx 1 lester lester      39 Jun  4 10:33 libRDKitChemicalFeatures.so.1 -> libRDKitChemicalFeatures.so.1.2024.03.3

@chryswoods: I'll PR to Sire devel to fix this. Given how long we've been building I'm surprised that we not had this issue before. (I checked older versions and the SONAME structure is the same.)

chryswoods
chryswoods previously approved these changes Jun 13, 2024
Copy link
Contributor

@chryswoods chryswoods left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍

@lohedges lohedges temporarily deployed to biosimspace-build June 14, 2024 15:18 — with GitHub Actions Inactive
@lohedges lohedges temporarily deployed to biosimspace-build June 14, 2024 15:18 — with GitHub Actions Inactive
@lohedges lohedges temporarily deployed to biosimspace-build June 14, 2024 15:18 — with GitHub Actions Inactive
@lohedges lohedges temporarily deployed to biosimspace-build June 14, 2024 15:18 — with GitHub Actions Inactive
@lohedges lohedges temporarily deployed to biosimspace-build June 14, 2024 15:18 — with GitHub Actions Inactive
@lohedges lohedges dismissed chryswoods’s stale review June 17, 2024 08:18

The merge-base changed after approval.

@lohedges lohedges merged commit 3df86f9 into devel Jun 17, 2024
5 checks passed
@lohedges lohedges deleted the fix_299 branch June 17, 2024 08:19
lohedges added a commit that referenced this pull request Jun 17, 2024
lohedges added a commit that referenced this pull request Jun 17, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[BUG] Unable to save Mol2 files through BioSimSpace
2 participants