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

Build (lint) is failing (ligand_neighbourhood_alignment) #1301

Open
alanbchristie opened this issue Jan 25, 2024 · 9 comments
Open

Build (lint) is failing (ligand_neighbourhood_alignment) #1301

alanbchristie opened this issue Jan 25, 2024 · 9 comments
Assignees
Labels
2024-06-14 mint Data dissemination 2 XChemAlign

Comments

@alanbchristie
Copy link
Collaborator

The ligand_neighbourhood_alignment repository CI build is failing (has been failing for some time). Before considering this code to be ready for production I think the CI build needs to be successful, even if this means pulling broken stuff out of it.

Broken builds should not be considered production ready.

@mwinokan mwinokan added 2024-03-15 indigo Data dissemination loose ends and removed 2023-11-02 yellow Too big for V2 labels Mar 13, 2024
@mwinokan mwinokan assigned mwinokan and tdudgeon and unassigned mwinokan Mar 13, 2024
@mwinokan
Copy link
Collaborator

@tdudgeon is this still failing?

@mwinokan mwinokan moved this to XChemAlign in Fragalysis May 29, 2024
@phraenquex phraenquex added 2024-06-14 mint Data dissemination 2 and removed 2024-03-15 indigo Data dissemination loose ends labels Jun 14, 2024
@tdudgeon
Copy link
Collaborator

Yes. @alanbchristie set this up so he will need to investigate.

@tdudgeon tdudgeon assigned alanbchristie and unassigned tdudgeon Jun 18, 2024
@alanbchristie alanbchristie moved this from XChemAlign to In Progress (DEV) in Fragalysis Jun 19, 2024
@alanbchristie
Copy link
Collaborator Author

There are a number of initial local development problems as it is difficult to develop on modern hardware. xchem align does not work for Python 3.12 (there is no distutils in Python 3.12) and rdkit fails to install on Apple M1 (ARM) hosts. The requires-python in the pyproject.toml probably needs to exclude Python 3.12 unless the developers think that 3.12 should be supported?

Also numpy was updated on 16th June (to version 2) and pandas appears to be ratehr sloppy in its requirements because it happily pulls in the latest numpy with the result that basic test fail, with the following error: -

ValueError: numpy.dtype size changed, may indicate binary incompatibility. Expected 96 from C header, got 88 from PyObject

(see https://stackoverflow.com/questions/78634235/)

We also need to try and fix build errors earlier because they have been failing now for more than 10 months?

@alanbchristie
Copy link
Collaborator Author

I’ve fixed the CI build and the only risk is the explicit requirement of the last v1 release of the numpy package (1.26.4). There was no explicit version in the project’s pyproject.toml but without it pandas pulls in v2.0.0, which breaks the code.

I have also removed the use of mypy. This has been broken for a long time and now issues more than 50 errors. If mypy remains a project requirement I suggest a specific issue is raised to adjust the code so that it passes the mypy tests (which may be a significant level of effort).

Changes (fixes) have been merged to the xchemn-align master branch.

Future breaks in the CI build must be treated with higher priority.

@alanbchristie alanbchristie moved this from In Progress (DEV) to Dev Done - Do review (DEV) in Fragalysis Jun 19, 2024
@mwinokan
Copy link
Collaborator

@alanbchristie has fixed the build (by disabling two tests) but those two tests will need fixing to be included in the CI build

@mwinokan mwinokan moved this from Dev Done - Do review (DEV) to In Progress (DEV) in Fragalysis Jun 20, 2024
@tdudgeon
Copy link
Collaborator

@ConorFWild is needed to get the tests working

@alanbchristie
Copy link
Collaborator Author

The failed tests are documented in the following xchem-align issues: -

@alanbchristie alanbchristie moved this from In Progress (DEV) to XChemAlign in Fragalysis Jun 25, 2024
@mwinokan mwinokan assigned tdudgeon and unassigned ConorFWild Sep 17, 2024
@mwinokan
Copy link
Collaborator

Now that XCA is stable it will be very useful to fix the automated testing to correctly flag errors from new dev

@alanbchristie
Copy link
Collaborator Author

alanbchristie commented Sep 18, 2024

After fixing what appeared to be the initial lint failure (related to the docs.yml workflow), and committed on branch issue_1301, what followed was a very large number of lint errors relating to the code. Sadly far too of these errors require code changes and I am not in a position to make any source code changes in this repository.

Someone who understands the code and its CI needs to fix this or we now need to switch the CI off altogether, and tackle the CI as a separate section of effort after the release. Be aware, as the CI has been broken for a very (very) long time this could take many days of effort to resolve.

Then, when done, fix CI issues when they arise.

Additionally, is pre-commit up to date on this repository and is it being used?

@alanbchristie alanbchristie moved this from In Progress (DEV) to XChemAlign in Fragalysis Sep 18, 2024
@alanbchristie alanbchristie changed the title Build (lint) is failing Build (lint) is failing (ligand_neighbourhood_alignment) Sep 18, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
2024-06-14 mint Data dissemination 2 XChemAlign
Projects
Status: mint
Development

No branches or pull requests

5 participants