Add files for PYPI by serhii73 · Pull Request #14 · bosswissam/pysize

serhii73 · 2018-10-01T19:10:52Z

Resolve #13

webknjaz

Factor out all unrelated changes into separate PRs.

webknjaz · 2018-10-01T20:30:11Z

.travis.yml

  - "nightly" # currently points to 3.6-dev
 # command to run tests
 script: nosetests
+notifications:


Don't modify notifications. It's not related, nor needed.

webknjaz · 2018-10-01T20:32:27Z

.travis.yml

+  email:
+    on_success: never
+    on_failure: always
+deploy:


Current implementation adds this step to all jobs in the matrix. The suggestion is to create an additional stage with a separate job for deployment. And then use if-clause to run that stage only for tagged commits.

webknjaz · 2018-10-01T20:33:30Z

.travis.yml

+  password:
+    secure: 
+ on:
+    branch: master


It shouldn't be triggered for every single commit in master. Only tagged builds should publish dist.

webknjaz · 2018-10-01T20:34:12Z

requirements.txt

@@ -0,0 +1 @@
+nose==1.3.7


How is this change related to the PR?

webknjaz · 2018-10-01T20:37:00Z

setup.py

-      version='1.0',
-      py_modules=['pysize'],
-      )
+with open("README.md", "r") as fh:


For Python 2 compat use io.open() with UTF-8 encoding. Also, newer setuptools allow using declarative setup.cfg, but that'd require upgrading setuptools in CI.

webknjaz · 2018-10-01T20:38:16Z

setup.py

+    version="1.0",
+    description="Use to quickly measure the size of your python objects.",
+    py_modules=["pysize"],
+    package_dir={"": "src"},


Why do you combine this with py_modules?

webknjaz · 2018-10-01T20:39:10Z

src/pysize.py

    # self-referential objects
    seen.add(obj_id)
-    if hasattr(obj, '__dict__'):
+    if hasattr(obj, "__dict__"):


This is unrelated

webknjaz · 2018-10-01T20:40:22Z

src/pysize.py

@@ -1,5 +1,6 @@
-import sys


Why did you move the module?

webknjaz · 2018-10-01T20:42:11Z

setup.py

+
+setup(
+    name="pysize",
+    version="1.0",


How about integrating setuptools-scm? Maybe in a follow up PR.

webknjaz · 2018-10-02T19:52:12Z

setup.py

+about = {}
+if not VERSION:
+    with open(os.path.join(here, NAME, '__version__.py')) as f:
+        exec(f.read(), about)


No, it's a horrible idea and not related to what I asked. https://github.com/pypa/setuptools_scm

webknjaz · 2018-10-02T19:53:21Z

__version__.py

@@ -0,0 +1,3 @@
+VERSION = (0, 0, 1)


This is not needed.

webknjaz · 2018-10-02T19:53:40Z

MANIFEST.in

@@ -0,0 +1 @@
+include README.md LICENSE


setuptools-scm takes care of manifest better.

webknjaz · 2018-10-09T17:33:27Z

setup.py

-      version='1.0',
-      py_modules=['pysize'],
-      )
+__requires__ = ('setuptools >=36.3.0', )


@bosswissam do you think you want to require this version of setuptools as a pre-requisite?

what's your recommendation? Sorry not super familiar with pypi setup.

Well, the question is what do you want for your users.

You're going to have a wheel (binary dist) and a tarball (source dist).
When building wheel, it's okay to upgrade setuptools in CI to use latest features.

But when it comes to users they may have older setuptools on their machines.
It doesn't affect wheel, because its installation is just unpacking a zip archive.
With sdist it's a bit more complicated: pip install actually executes setup.py install under the hood. So all those options/meta stored in setup.cfg will simply not be read. Of course, I've got a shim for that if needed.

But most of the users will install from wheel anyway.

The decision is up to you.

@bosswissam so?

@bosswissam ^

webknjaz · 2018-10-13T00:54:17Z

setup.cfg

+author = bosswissam
+author_email = bosswissam@gmail.com
+description = Use to quickly measure the size of your python objects.
+long_description = file: README.md


@bosswissam is it okay if we convert README into rst?

@serhii73 if we'll decide to keep Markdown, it'll require one extra option to be set in metadata, explicitly declaring the correct MIME type of the description.

webknjaz · 2018-10-14T21:18:05Z

.travis.yml

+    dist: xenial
+    sudo: true
+
+  - python: 3.5


All Pythons except for 3.7 don't need sudo, nor they need xenial. Revert this change and only leave 3.7 inside of jobs.include

webknjaz · 2018-10-14T21:19:18Z

setup.cfg

+[options]
+python_requires = >=2.6
+install_requires =
+    nose


I believe it's not a runtime requirement. So you have to remove it.

webknjaz · 2018-10-14T21:23:28Z

README.rst

+pysize
+======
+
+Use to quickly measure the size of your python objects. Supports: \* Measuring the size of self-referential objects \* No double-counting for repeated objects in a collection \* Python v2/v3


This list is broken.

webknjaz · 2018-10-14T22:03:29Z

README.rst

+.. code:: python
+
+    >>> class Test(object):
+    >>>    pass


>>> -> ...

webknjaz · 2018-10-14T22:03:41Z

README.rst

+    >>> get_size(y)
+    80971
+
+To measure the size of ``properties``, call ``pysize.get_size`` on the full list of the object's members minus overhead and unwanted memberes:


-> members

webknjaz · 2018-10-14T22:04:13Z

README.rst

+License
+=======
+
+`MIT <LICENSE.txt>`__


I believe it needs a single trailing underscore.

webknjaz · 2018-10-14T22:04:29Z

README.rst

+License
+=======
+
+`MIT <LICENSE.txt>`__


Also, it's not txt anymore, but rst

webknjaz · 2018-10-17T12:12:46Z

.travis.yml

@@ -1,12 +1,12 @@
-conditions: v1


Why did you remove it? you should add it back!

webknjaz · 2018-10-17T12:13:00Z

.travis.yml


-script: nosetests
+install:
+- pip install pytest==3.9.1


This change is unrelated.

webknjaz

Get back conditions opt-in and revert unrelated test runner change.

webknjaz

It's not about removing the existing things, but adding what's stated in the PR title.

webknjaz · 2018-10-18T12:29:26Z

.travis.yml

 language: python
+
 python:
-  - "2.6"


Get back all of the matrix jobs

webknjaz · 2018-10-18T12:32:57Z

setup.cfg

+license_file = LICENSE.rst
+keywords =
+    check
+    Python


this keyword is useless.

webknjaz · 2018-10-18T19:20:07Z

.travis.yml

-  - "3.5-dev" # 3.5 development branch
-  - "nightly" # currently points to 3.6-dev
-# command to run tests
+- 2.6


Keep the original syntax, it shouldn't appear in this diff.

(refactoring is not related to the PR purpose)

webknjaz · 2018-10-18T19:20:52Z

.travis.yml

+- 3.4
+- 3.5
+- 3.6
+- 3.7


It's already in jobs.include + it doesn't work like others and will error out in Travis CI.

Suggested change

- 3.7

webknjaz · 2018-10-18T19:25:22Z

.travis.yml

+        tags: true
+      user: bosswissam
+      password:
+        secure: 


@bosswissam you should use the command travis encrypt to encrypt your password and put the result in here.

@bosswissam ^

webknjaz · 2018-10-18T19:40:39Z

.travis.yml

+- 3.4
+- 3.5
+- 3.6
+- 3.7


Look at https://travis-ci.org/bosswissam/pysize/builds/443226261. There's two jobs for 3.7. The first one (failing) is this line and the second one is one in jobs.include, which is configured properly.

The way it works is that Travis CI generates a matrix of jobs from python: section and for each of them it uses script:/install: from the root level of this config.

Jobs in jobs.include inherit all of the steps from the root level, but when you explicitly specify different values for things those override inherited settings.

So for the failing job the config is:

dist: trusty # implicit default sudo: false # implicit default language: python # specified explicitly python: 3.7 # specified explicitly install: pip install -r requirements.txt # implicit default script: nosetests # specified explicitly

And for succeeding job it's:

dist: xenial # specified explicitly, overrides value from config root level sudo: true # specified explicitly, overrides value from config root level python: 3.7 # specified explicitly, overrides value from config root level install: pip install -r requirements.txt # inherited from config root script: nosetests # inherited from config root

P.S. If you are not sure how the config you wrote has been parsed by Travis, there's a "View config" tab in each job, like:

https://travis-ci.org/bosswissam/pysize/jobs/443226269/config

https://travis-ci.org/bosswissam/pysize/jobs/443226270/config

webknjaz · 2018-10-18T19:45:23Z

setup.cfg

@@ -0,0 +1,45 @@
+[metadata]
+name = pysize


Project/package distribution with this name already exists on PyPI: https://pypi.org/p/pysize. The upload will fail should you try to reuse it.
Let's use smth else like pysize-inspector.

Suggested change

name = pysize

name = pysize-inspector

cc @bosswissam ^

webknjaz · 2018-10-19T09:20:22Z

.travis.yml

@@ -10,3 +11,30 @@ python:
  - "nightly" # currently points to 3.6-dev


I think it's okay to replace 3.5-dev with 3.6 in this PR, so that we'd have the whole version coverage.

P.S. nightly points to 3.8-dev now

webknjaz · 2018-10-19T09:22:11Z

README.rst

+|Build Status|
+
+pysize-inspector
+======


Underline should match the test length.

webknjaz · 2018-10-19T09:22:26Z

setup.cfg

@@ -0,0 +1,45 @@
+[metadata]
+name = pysize-inspector


@bosswissam ^

webknjaz · 2018-10-22T14:42:20Z

.travis.yml

+    sudo: true
+
+  - stage: &stage_deploy_name upload new version of python package to PYPI (only for tagged commits)
+    name: "Deploy"


Quotes are not really needed for strings unless you use some special characters there, which you don't.

webknjaz

LGTM

@bosswissam would you please add encrypted PyPI creds to Travis CI config?

bosswissam · 2018-10-22T16:51:07Z

Sorry for the delay on my end. This is on my radar, just need some time to make sure I'm making the right decision re setup.py & pypi naming.

webknjaz · 2018-10-22T18:02:26Z

@bosswissam maybe we'd just drop in the shim for supporting fairly old setuptools? This would reduce the number of questions :)

serhii73 added 3 commits October 1, 2018 22:07

Update format test

63dd152

Add requirements

bc247ec

Update files for pypi

cafe2ef

serhii73 mentioned this pull request Oct 1, 2018

Please publish this to PyPI #13

Closed

webknjaz suggested changes Oct 1, 2018

View reviewed changes

serhii73 added 8 commits October 2, 2018 21:47

Remove requirements

2d2e4fe

Remove notifications

328a53f

Deploying tags

9fc697c

Add manifest

ae20d61

Update setup

1e2d963

Come back code pysize

4180d43

Update setup

6a6ab01

Come back test

5832152

webknjaz reviewed Oct 2, 2018

View reviewed changes

__version__.py Outdated

@@ -0,0 +1,3 @@

VERSION = (0, 0, 1)

Copy link

webknjaz Oct 2, 2018

Choose a reason for hiding this comment

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

This is not needed.

webknjaz reviewed Oct 2, 2018

View reviewed changes

MANIFEST.in Outdated

@@ -0,0 +1 @@

include README.md LICENSE

Copy link

webknjaz Oct 2, 2018

Choose a reason for hiding this comment

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

setuptools-scm takes care of manifest better.

serhii73 added 2 commits October 9, 2018 18:52

Update setup.py and add setup.cfg

e05fc0e

Update setup.py and add setup.cfg

f195d4f

webknjaz reviewed Oct 9, 2018

View reviewed changes

webknjaz reviewed Oct 13, 2018

View reviewed changes

serhii73 added 2 commits October 14, 2018 21:46

Convert md to rst 📝

e3b2d71

Add travis

26411bb

webknjaz reviewed Oct 14, 2018

View reviewed changes

README.rst Outdated

.. code:: python

>>> class Test(object):

>>> pass

Copy link

webknjaz Oct 14, 2018

Choose a reason for hiding this comment

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

>>> -> ...

webknjaz reviewed Oct 14, 2018

View reviewed changes

README.rst Outdated

License

=======

`MIT <LICENSE.txt>`__

Copy link

webknjaz Oct 14, 2018

Choose a reason for hiding this comment

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

I believe it needs a single trailing underscore.

webknjaz reviewed Oct 14, 2018

View reviewed changes

README.rst Outdated

License

=======

`MIT <LICENSE.txt>`__

Copy link

webknjaz Oct 14, 2018

Choose a reason for hiding this comment

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

Also, it's not txt anymore, but rst

🔨

d524f4c

webknjaz reviewed Oct 17, 2018

View reviewed changes

.travis.yml Outdated

@@ -1,12 +1,12 @@

conditions: v1

Copy link

webknjaz Oct 17, 2018

Choose a reason for hiding this comment

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

Why did you remove it? you should add it back!

webknjaz reviewed Oct 17, 2018

View reviewed changes

.travis.yml Outdated

script: nosetests

install:

- pip install pytest==3.9.1

Copy link

webknjaz Oct 17, 2018

Choose a reason for hiding this comment

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

This change is unrelated.

webknjaz suggested changes Oct 17, 2018

View reviewed changes

serhii73 added 5 commits October 18, 2018 11:43

Remove sudo and xenial

dda5feb

Remove install_requires from setup.cfg

fc7c1d9

Add nosetests to travis and remove pytest

37d1f64

Add conditions: v1

183ea3e

Update travis

a8ce71c

webknjaz suggested changes Oct 18, 2018

View reviewed changes

.travis.yml

language: python

python:

- "2.6"

Copy link

webknjaz Oct 18, 2018

Choose a reason for hiding this comment

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

Get back all of the matrix jobs

webknjaz reviewed Oct 18, 2018

View reviewed changes

setup.cfg Outdated

license_file = LICENSE.rst

keywords =

check

Python

Copy link

webknjaz Oct 18, 2018

Choose a reason for hiding this comment

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

this keyword is useless.

serhii73 added 2 commits October 18, 2018 17:48

Add python versions to travis

6c04825

Remove not need keyword

c09c09a

webknjaz reviewed Oct 18, 2018

View reviewed changes

serhii73 added 2 commits October 19, 2018 10:29

Keep the original syntax

982182c

Rename to pysize-inspector

79ac141

webknjaz reviewed Oct 19, 2018

View reviewed changes

README.rst Outdated

|Build Status|

pysize-inspector

======

Copy link

webknjaz Oct 19, 2018

Choose a reason for hiding this comment

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

Underline should match the test length.

webknjaz reviewed Oct 19, 2018

View reviewed changes

setup.cfg

@@ -0,0 +1,45 @@

[metadata]

name = pysize-inspector

Copy link

webknjaz Oct 19, 2018

Choose a reason for hiding this comment

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

@bosswissam ^

serhii73 added 3 commits October 22, 2018 16:27

Fix underline

6e194ce

Replace python versions in travis

8ae6013

Del not need comment

966825f

webknjaz reviewed Oct 22, 2018

View reviewed changes

webknjaz approved these changes Oct 22, 2018

View reviewed changes

Remove quotes

ca3f9e1

		@@ -10,3 +11,30 @@ python:
		- "nightly" # currently points to 3.6-dev

		@@ -0,0 +1 @@
		nose==1.3.7

Conversation

serhii73 commented Oct 1, 2018

Uh oh!

webknjaz left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

webknjaz left a comment

Choose a reason for hiding this comment

Uh oh!

webknjaz left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

webknjaz Oct 18, 2018 •

edited

Loading