-
-
Notifications
You must be signed in to change notification settings - Fork 5k
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
add datreant recipes. #1144
add datreant recipes. #1144
Conversation
Hi! This is the friendly automated conda-forge-linting service. I wanted to let you know that I linted all conda-recipes in your PR ( Here's what I've got... For recipes/datreant:
For recipes/datreant.core:
|
Hi! This is the friendly automated conda-forge-linting service. I wanted to let you know that I linted all conda-recipes in your PR ( Here's what I've got... For recipes/datreant:
|
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 ( |
ping @datreant/coredevs, @dotsdl as well. |
Awesome! I'm really curious if this works on Windows as written (AppVeyor). Thanks @kain88-de! This is a big deal. |
I'm not sure why the datreant package can't be build it works locally with |
I'm having a look at all the CI results now. Not sure what to do with CircleCI. the Linux Travis build worked, but not OSX. Also Windows (AppVeyor) failed. Do you have a Mac? I don't have a Windows machine lying around; not sure what people do to troubleshoot Windows builds. |
|
||
source: | ||
git_url: https://github.com/datreant/datreant.core.git | ||
git_tag: release-{{ version }} |
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.
We prefer to use tarballs. Could we please switch this over to one?
So it's the namespace package that isn't building. On both OSX and Windows we get something like:
|
Yep, this appears to be a |
Thanks @jakirkham for bringing this to the attention of the upstream tools. Is there anything we can do as a workaround, or would you suggest waiting for now? |
Let's wait and see what upstream says. Normally the developer is very responsive and he's an admin at conda-forge. |
Hi! This is the friendly automated conda-forge-linting service. I wanted to let you know that I linted all conda-recipes in your PR ( Here's what I've got... For recipes/datreant.core:
|
I've created a commit to restart the travis build as this should be fixed with the latest conda-build-all. |
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 ( |
This currently fails because the |
This package currently doesn't work on windows. We would need to make some major changes to make it work under windows. Will that be a problem? |
There are a number of packages on conda-forge that do not currently build on windows (here's a rough list), so that's fine. Just add: skip: True # [win] to the build section |
|
||
extra: | ||
recipe-maintainers: | ||
- datreant/coredevs |
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.
Ah, so, I don't think this would work. Though I'm actually a little unsure. It is better to list explicitly when members are maintainers by their handles.
Fixed all comments. Seriously thanks for all the help. |
There is something odd. The logs on circleci show an error
Still it is reported everything has passed. This likely isn't important for this package since the failure occurs when I run the unit-test packages with datreant.core. Still I find it strange that an error is raised an the build succeds |
The builds are passing now. Anything else that needs to be done? |
|
||
build: | ||
number: 1 | ||
skip: True # [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.
We are skipping windows but have a bld.bat
?
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.
sorry forget to remove that when I deactivated the windows build.
This includes two recipes for datreant. One is the empty namespace package 'datreant' and the other contains the actual core pacakge for datreant. The config for datreant itself is to allow to install a simple namespace package. For 'datreant.core' the extra build scripts are needed to remove the '*-nspkg.pth' file produced by setuptools, this is to have anaconda-verify pass with the package.
weird that here the package name is different from the repository name.
remove anchor and deactivate build for windows
all checks pass now again after a rebase against master. |
|
||
about: | ||
home: http://datreant.org/ | ||
license: BSD 3-Clause |
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.
Please ask upstream to include the license file in MANIFEST.in
. Also please note the issue/PR here.
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.
Why should the license file be included in the MANIFEST.in
? I haven't worked with that file yet. We note the license in the setup.py.
Also this change in the license will only be released in the next version.
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.
It ensures that the license file shows up in sdist
s and other packages generated from the source. This is important as it is required by the BSD license. Further it will make it easy for this package to comply with the license as we can do something like license_file: LICENSE
to include it here.
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.
OK I understand. I'll add the files. This won't be released until version 0.8 though which is still some at some unknown time in the future.
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.
Created Issue datreant/datreant#94
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.
Of course. We are just trying to improve compliance on this front.
@jakirkham I think this looks all good to me as long as we are happy with the namespace story? I'm not aware of our hosting of any other namespace packages so I'm reluctant to pull this in myself without one last second pair of eyes. |
Agreed. I think I'm also happy with this. The namespace recommendation seems to come from @ilanschnell. Given they are doing this with the |
Ping |
I have no experience with namespace packages, but have discussed this with @ilanschnell. It's worth a try here. |
Thanks @msarahan! |
Thanks |
This includes two recipes for datreant. One is the empty namespace
package 'datreant' and the other contains the actual core pacakge
for datreant. The config for datreant itself is to allow to install
a simple namespace package. For 'datreant.core' the extra build scripts
are needed to remove the '*-nspkg.pth' file produced by setuptools,
this is to have anaconda-verify pass with the package.
I know this deviates from the usual structure that you prefer for normal packages. But the namespace package means it has to be treated differently.
Another question I have. This introduces two packages with one (datreant) basically being empty.
I currently decided to pack both into one PR to give some context. But I can split them up if that is preferred.