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 pythonPackage.wrf-python #48835

Merged
merged 1 commit into from Feb 17, 2019
Merged

add pythonPackage.wrf-python #48835

merged 1 commit into from Feb 17, 2019

Conversation

mhaselsteiner
Copy link
Contributor

Added python library for post processing of WRF model output.

Remark

naming convention violates pep 0503 to be consistent with pypi.

@costrouc
Copy link
Member

costrouc commented Oct 22, 2018

Also please change commit message to pythonPackages.wrf-python: init at 1.2.0 to follow nixpkgs PR naming conventions.

xarray
];

doCheck = false;
Copy link
Member

Choose a reason for hiding this comment

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

Why are tests not being run? If they are not included with the release please specify in a comment.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

In the setup.py no test suite is specified and and there is also no runtests.py . Some tests are included but some dependencies on files which are not included, probably due to their size.
I will add a comment

description = "WRF postprocessing library for Python";
homepage = http://wrf-python.rtfd.org;
license = lib.licenses.asl20;
platforms = lib.platforms.all;
Copy link
Member

Choose a reason for hiding this comment

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

No need for platforms = platforms.all this is controlled by buildPythonPackage

propagatedBuildInputs = [
setuptools
wrapt
numpy>=1.11
Copy link
Member

Choose a reason for hiding this comment

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

In nix you cannot specify version. We try to make everything use the latest version. numpy>=1.11 -> numpy

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ok, thanks. I realized, when testing it again this caused my build to fail. Sorry, it's my first time contributing here..

@mhaselsteiner
Copy link
Contributor Author

Thanks for the feedback. I will try to apply the changes asap.

@mhaselsteiner
Copy link
Contributor Author

I implemented all the changes. Hope it's ok now. Let me know, if there is still something not aligned with the guidelines.

sha256 = "0maphsqfnrqpp52zz7xwcfjhw7y0fvq81mz91a9rfqvw0b2ww83m";
};
propagatedBuildInputs = [
gfortran
Copy link
Member

Choose a reason for hiding this comment

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

Is gfortran needed for running the package? If is only a build requirement https://github.com/NCAR/wrf-python/blob/master/setup.py#L25-L45 then it should be specified in buildInputs = [ ].

@costrouc
Copy link
Member

costrouc commented Nov 7, 2018

Please move to gfortran to buildInputs = [ ] if it is only a build dependency. Otherwise looks great to me!

@Ekleog
Copy link
Member

Ekleog commented Jan 27, 2019

@mhaselsteiner Are you still interested in pushing this forward? It should be just a one-line change, but now there is a merge conflict, so it'll maybe be a bit harder.

@mhaselsteiner
Copy link
Contributor Author

I resolved the conflict and added some tests

@ryantm
Copy link
Member

ryantm commented Feb 17, 2019

@GrahamcOfBorg build pythonPackages.wrf-python

@ryantm ryantm merged commit cea567c into NixOS:master Feb 17, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants