-
Notifications
You must be signed in to change notification settings - Fork 13
Packaging of Graphium 3.0 #531
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
base: graphium_3.0
Are you sure you want to change the base?
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I have made a few minor requests.
Also, I feel that the Graphium-cpp main page should not be empty. Perhaps a text stating:
Welcome to the documentation for the C++ API of the graphium library. For the python API, go to this linkhttps://graphium-docs.datamol.io/stable/). The C++ API is used to significantly accelerate the computation of positional and structural encodings, but also the caching of the supervised labels into very light and optimized files. This significantly accelerates the dataloading and removes the need for long pre-processing of large datasets.
README.md
Outdated
| ### conda-forge | ||
| conda-forge is the recommended method for installing Graphium. To install Graphium via conda-forge, run the following command: | ||
| ``` | ||
| conda install graphium |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| conda install graphium | |
| mamba install graphium -c conda-forge |
env.yml
Outdated
|
|
||
| # scientific | ||
| - numpy == 1.26.4 | ||
| - numpy=1.25.0 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| - numpy=1.25.0 | |
| - numpy <= 1.25.0 |
Too strict
env.yml
Outdated
|
|
||
| # chemistry | ||
| - rdkit == 2024.03.4 | ||
| - rdkit=2024.03.4 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Too strict. Can you do <= or >=?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'll run a test build with <= and report back.
19e4dca to
55b4076
Compare
533c770 to
90dc934
Compare
474c802 to
3a10b45
Compare
|
@DomInvivo applied your suggestions and got the tests running for all operating systems we'll support. It's ready for another quick round of review. Please remember to build the docs and checkout the changes I made to the C++ API docs. |

Changelogs
setup.pyto buildgraphium_cppas part of thegraphiumbuild process.setup.pyto allow propergraphium_cppcompilation.doxygenforgraphium_cppdoc building. Configured to integrate into existing docs deployment.(@DomInvivo I would pull this branch and runmkdocs serveto ensure it all looks good to you. There is a nav menu button named "Graphium C++").env.ymlfile updated with dependencies/versions that are compatible withgraphiumandgraphium_cpp.README.mdto reflect how to install3.0.0. A note was added about PyPi version limitation.Checklist:
feature,fixortest(or ask a maintainer to do it for you).discussion related to that PR
Important Notes
graphiumdocumentation given that he has the most context about what features are available and not in the package. We looked today and some work was needed.graphium/trainer/metrics.py. Is this correct?3.9,3.10and3.11right now. Becausepygis pinning us tonumpyversion1.25.0, we cannot use3.12(not compatible with that version ofnumpy). Oncepygmakes a new release, I will update our conda-forge recipe to build for3.12.tests/test_datamodule.py. I had to update two assert statements to get passing tests. Was this update correct or did I just make a failing test pass?doxygen.