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

ADD: MDsrv version 0.3.5 #3553

Merged
merged 5 commits into from
Aug 14, 2017
Merged

ADD: MDsrv version 0.3.5 #3553

merged 5 commits into from
Aug 14, 2017

Conversation

j0kaso
Copy link
Contributor

@j0kaso j0kaso commented Aug 14, 2017

No description provided.

@conda-forge-linter
Copy link

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 (recipes/mdsrv) and found it was in an excellent condition.


build:
number: 0
script: python setup.py install
Copy link
Member

Choose a reason for hiding this comment

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

Please use:

script: python setup.py install --single-version-externally-managed --record record.txt

https://github.com/conda-forge/staged-recipes/blob/master/recipes/example/meta.yaml#L26

since it appears that this package uses setuptools.

extra:
recipe-maintainers:
- j0kaso
- arose
Copy link
Member

Choose a reason for hiding this comment

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

@arose -- are you ok being added as a co-maintainer of the MDsrv package on conda-forge? Your name was added by @j0kaso. Being a co-maintainer would give you write permission to the github repository allowing you to update the package on conda-forge.

Copy link

Choose a reason for hiding this comment

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

yes, I would like to be added as a co-maintainer, thanks


run:
- python
- setuptools
Copy link
Member

Choose a reason for hiding this comment

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

Is setuptools an actual runtime dependency?

url: https://pypi.io/packages/source/{{ name[0] }}/{{ name }}/{{ name }}-{{ version }}.tar.gz
sha256: {{ sha256 }}

build:
Copy link
Member

Choose a reason for hiding this comment

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

Looking at the package's setup.py it appears that there is a python entry_point. Please add it here:

https://conda.io/docs/building/meta-yaml.html#python-entry-points

https://github.com/arose/mdsrv/blob/master/setup.py#L47-L48

- mdanalysis # [not win and py2k]

test:
imports:
Copy link
Member

Choose a reason for hiding this comment

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

Please test the command line interface:

https://conda.io/docs/building/meta-yaml.html#test-commands


about:
home: https://github.com/arose/mdsrv
license: MIT
Copy link
Member

Choose a reason for hiding this comment

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

Please add: license_file: LICENSE

Copy link
Contributor Author

@j0kaso j0kaso left a comment

Choose a reason for hiding this comment

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

Thanks for the advice @synapticarbors ! Changed accordingly within the last commit.

number: 0
script: python setup.py install --single-version-externally-managed --record record.txt
entry_points:
-mdsrv = mdsrv:entry_point
Copy link
Member

Choose a reason for hiding this comment

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

I think you need a space between the - and mdsrv. I think this is what's causing the current failure.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

indeed - thanks!

@synapticarbors synapticarbors merged commit 567ffa5 into conda-forge:master Aug 14, 2017
@synapticarbors
Copy link
Member

@j0kaso -- Thanks for your contribution to conda-forge. A feedstock for this recipe should be generated soon and you'll be receiving several automated messages about it. When the feedstock has been rendered, please take a look at the README in that repo for information about updating and maintaining the recipe. One of the key points is all updates should be done by forking that repo and following a standard pull request workflow. If you haven't looked at it already the docs are quite helpful:

https://conda-forge.org/docs/

If you run into any difficulties please let us know.

@synapticarbors
Copy link
Member

Also, while I merged the recipe, I'm still a bit unsure about a lot of the build requirements. Some are there because of the packages listed in the setup.py under setup_requires, but some of the others I'm not sure about (namely zlib, msinttypes). There shouldn't be any harm leaving them in there, but if they aren't necessary, they should be dropped in a future update to the feedstock. Also upstream, maybe the setup_requires should be reviewed since it doesn't look like scipy is used anywhere and I don't see anything that would require cython.

@j0kaso
Copy link
Contributor Author

j0kaso commented Aug 14, 2017

Thanks @synapticarbors for the help & for merging into conda-forge. I'm happy to have a look at the feedstock soon and will update our documentation to have conda-forge as the default channel as soon as it's available.
Regarding the build and setup requires, most of them (cython, scipy, zlib and msinttype - the latter only for windows) are needed in order to install mdtraj but I will test with focus on this if really all of them are required explicitly (nglviewer/mdsrv#31).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

4 participants