-
-
Notifications
You must be signed in to change notification settings - Fork 21
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
Remove rdkit-dev #103
base: main
Are you sure you want to change the base?
Remove rdkit-dev #103
Conversation
Hi! This is the friendly automated conda-forge-linting service. I just wanted to let you know that I linted all conda-recipes in your PR ( |
I'm wondering if this change could also indirectly address #60 |
I think this looks good? Just to confirm - It looks like this keeps the Also, is there any way to see the package size before/after this change from the CI logs? Or would someone have to build locally to check? |
- name: rdkit-dev | ||
requirements: | ||
run: | ||
- rdkit |
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.
- name: rdkit-dev | |
requirements: | |
run: | |
- rdkit | |
- name: rdkit-dev | |
build: | |
run_exports: | |
- {{ pin_subpackage('rdkit', max_pin='x.x.x') }} | |
requirements: | |
run: | |
- {{ pin_subpackage('rdkit', exact=True) }} |
We need the dev/non dev package versions to always match exactly because rdkit-dev is now an alias.
@@ -12,7 +12,7 @@ source: | |||
sha256: e2832077e258bfe906c9c0dc1664d2ba4fc0acf98bfe073c6383cb1d051b6ef0 | |||
|
|||
build: | |||
number: 0 | |||
number: 1 |
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.
number: 1 | |
number: 1 | |
run_exports: | |
- {{ pin_subpackage('rdkit', max_pin='x.x.x') }} |
If people link to the C API of this package, they need to get the same ABI at runtime.
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.
This change is for the overall build and will update the rdkit
subpackage as well; I don't think the pin is necessary and it seems a bit out of place since it's at a higher level than the subpackage definition?
@@ -51,6 +51,7 @@ requirements: | |||
test: | |||
commands: | |||
- python -c "import rdkit; assert rdkit.__version__ == '{{ version }}'" | |||
- if not exist %LIBRARY_INC%\\rdkit\\Catalogs\\Catalog.h exit 1 # [win] |
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.
Add more existence tests to ensure that all the expected outputs (types) are being included in the new combined package.
@mcs07, you can use the https://conda-forge.org/docs/maintainer/conda_forge_yml.html?highlight=conda+forge+yml#azure @conda-forge/rdkit |
Can we close this? |
rdkit-dev was created because the Windows package was too big. #86 seems to have made the Windows build lighter.
So rdkit-dev might not be useful anymore
Checklist
0
(if the version changed)conda-smithy
(Use the phrase@conda-forge-admin, please rerender
in a comment in this PR for automated rerendering)