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.

Sign up for GitHub

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

Jump to bottom

pythonPackages.typeguard: init at 2.3.1 #25040

Merged
merged 1 commit into from Apr 21, 2017
Merged

pythonPackages.typeguard: init at 2.3.1 #25040

merged 1 commit into from Apr 21, 2017

Conversation

obadz
Copy link
Contributor

@obadz obadz commented Apr 19, 2017 •
edited

Motivation for this change

Add typeguard. First time I contribute a python package so wanted to get another pair of eyes before I get too comfortable with merging these things.

Things done
  • Tested using sandboxing
    (nix.useSandbox on NixOS,
    or option build-use-sandbox in nix.conf
    on non-NixOS)
  • Built on platform(s)
    • NixOS
    • macOS
    • Linux
  • Tested compilation of all pkgs that depend on this change using nix-shell -p nox --run "nox-review wip"
  • Tested execution of all binary files (usually in ./result/bin/)
  • Fits CONTRIBUTING.md.

@mention-bot
Copy link

@obadz, thanks for your PR! By analyzing the history of the files in this pull request, we identified @FRidh to be a potential reviewer.

@obadz
Copy link
Contributor Author

obadz commented Apr 19, 2017

Well looks like I had contributed 44157c6 too but that was eons ago…

Mic92
Mic92 requested changes Apr 20, 2017
View reviewed changes
pkgs/top-level/python-packages.nix Outdated
@@ -23151,6 +23151,26 @@ in {
};
};

typeguard = buildPythonPackage rec {
Copy link
Member

@Mic92 Mic92 Apr 20, 2017 •
edited

Choose a reason for hiding this comment

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

Due the size of this file, we add all new/updated python packages to python-modules and only reference them here.

Copy link
Member

Choose a reason for hiding this comment

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

There are test, but they are probably not available on pypi. If this is the case we usually download from github instead.
They can be run by adding pytest to buildInputs and override checkPhase:

checkPhase = "py.test .";

pkgs/top-level/python-packages.nix Outdated
name = "typeguard-${version}";
version = "2.1.3";

src = pkgs.fetchurl {
Copy link
Member

Choose a reason for hiding this comment

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

fetchpypi can be used here.

FRidh
FRidh reviewed Apr 20, 2017
View reviewed changes
pkgs/top-level/python-packages.nix Outdated
};

propagatedBuildInputs = with self; [
setuptools_scm
Copy link
Member

Choose a reason for hiding this comment

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

this is typically only a build-time dependency and not a run-time dependency.

@obadz
Copy link
Contributor Author

obadz commented Apr 20, 2017

Thanks all for the feedback. How does the latest commit look?

FRidh
FRidh reviewed Apr 20, 2017
View reviewed changes
pkgs/top-level/python-packages.nix
@@ -23178,7 +23180,6 @@ in {
runsnakerun = buildPythonPackage rec {
name = "runsnakerun-2.0.4";


Copy link
Member

Choose a reason for hiding this comment

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

note this unrelated change

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yes there was an extra empty line…

@FRidh
Copy link
Member

FRidh commented Apr 20, 2017

looks good to me

@obadz
Copy link
Contributor Author

obadz commented Apr 20, 2017 •
edited

By the way, after running the tests, I see this exception/stack trace but not sure why. py.test still succeeds though (returns 0):

============================= test session starts ==============================
platform linux -- Python 3.5.3, pytest-3.0.6, py-1.4.31, pluggy-0.4.0
rootdir: /tmp/nix-build-python3.5-typeguard-2.1.3.drv-0/typeguard-2.1.3, inifile: setup.cfg
collected 112 items

tests/test_typeguard.py ................................................................................................................

========================== 112 passed in 0.47 seconds ==========================
Exception ignored in: <function WeakValueDictionary.__init__.<locals>.remove at 0x7ffff3495ae8>
Traceback (most recent call last):
  File "/nix/store/76rgpkm1jqgqwz4bjnk8y37z29q82zgy-python3-3.5.3/lib/python3.5/weakref.py", line 117, in remove
TypeError: 'NoneType' object is not callable
Exception ignored in: <function WeakValueDictionary.__init__.<locals>.remove at 0x7ffff3495ae8>
Traceback (most recent call last):
  File "/nix/store/76rgpkm1jqgqwz4bjnk8y37z29q82zgy-python3-3.5.3/lib/python3.5/weakref.py", line 117, in remove
TypeError: 'NoneType' object is not callable
Exception ignored in: <function WeakValueDictionary.__init__.<locals>.remove at 0x7ffff3495ae8>
Traceback (most recent call last):
  File "/nix/store/76rgpkm1jqgqwz4bjnk8y37z29q82zgy-python3-3.5.3/lib/python3.5/weakref.py", line 117, in remove
TypeError: 'NoneType' object is not callable
/nix/store/2b8jvmvv1xp0pprx767v8jjby0q67pax-python3.5-typeguard-2.1.3

@FRidh
Copy link
Member

FRidh commented Apr 21, 2017

Yes, I noticed it as well. It seems like they're not using the weak references correctly. I suppose upstream already knows about this?

@obadz obadz mentioned this pull request Apr 21, 2017
Closed
@obadz
Copy link
Contributor Author

obadz commented Apr 21, 2017 •
edited

Posted upstream here agronholm/typeguard#8 (comment). Merging this for now.

@obadz obadz merged commit dbb06ea into NixOS:master Apr 21, 2017
@obadz obadz deleted the python-typeguard branch April 21, 2017 20:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@FRidh FRidh FRidh left review comments

@Mic92 Mic92 Mic92 requested changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
4 participants
@obadz @mention-bot @FRidh @Mic92