proper Python packaging about veering HOT 37 CLOSED

Sure - I don't see why not... the code base is actively being worked on and we are not organized enough to have numbered releases or anything like that - would there be any issue with updating the pip version of the code?

from veering.

Releasing on pypi is optional. With a proper Python package setup one can install the development version with the command pip install git+https://github.com/henryseg/Veering. That would already be a step forward for having people easily play with the code.

from veering.

Releasing on pypi without versioning is not an option.

from veering.

Dear Vincent -

I am going to assume that Henry and Anna are fine with this, so yes, you have a green light.
I suggest the version number 0.0.0 so we can use pip install.
Tell me what I need to do...

best,

saul

from veering.

Ah, Henry, you need to do this - go to the Veering main page, click on "Settings"(last on horizontal list of items), click on "Access" (in sidebar), click on "Collaborators & teams", and to the right of "Manage access", click "Add people or Add teams". Add videlec as an admin (if you can...)

from veering.

from veering.

There are many huge files (> 1M) containing data. This is not convenient for (light) packaging. The list is

1,8M layered_one_cusp.txt
3,0M veering_layered.txt
4,1M veering_shapes_up_to_twelve_tetrahedra.pkl
5,1M veering_census.txt
8,0M veering_census_with_data.txt
70M veering_flow_graphs.txt

Also these .pkl and .txt files are spread in the main folder Veering and some other are in Veering/Data. Which one are relevant for packaging ?

from veering.

veering_census.txt and veering_census_with_data.txt are the most important. Perhaps we could have a secondary "More_Data" folder, which isn't included in the standard install for the other files?

from veering.

Let me start with these two files.

from veering.

I pushed my changes on the setup branch. If you are fine with it I can merge it in master.

from veering.

from veering.

Of course.

Though there is still an issue with markdown rendering. distutils setup ignores long_description_content_type while setuptools is half broken. Could I changed the README syntax from markdown to rst ? It will make the packager life easier.

from veering.

from veering.

It turns out that distutils is deprecated (and setuptools should be used). Python documentation is very much unclear about that. Markdown should be is perfectly fine.

from veering.

It sounds like you are saying "Saul, you don't have to do anything." which makes me very happy.

from veering.

The changes in the setup branch are not backward compatible as the code uses a lot of relative paths such as https://github.com/henryseg/Veering/blob/setup/veering/boundary_triangulation_with_snappy.py#L11-L12. Also many files have main functions which is a weird thing for a Python library.

I am not sure what to do about that.

from veering.

I tend to use the main() functions as working notepads for what we are trying to calculate with a file, separate from the (ideally) stable rest of each .py file. I'd rather not do away with them entirely, as it's a lot easier to remember how to get the code working with examples of what we were doing in the main(). I could have entirely separate files for working, although that would increase the number of files floating around.

I suspect that Saul's style of using the sage command line to calculate with is probably more standard, and doesn't leave main() functions in the code. The downside with this as I see it is that the code he writes as he is calculating is often not saved, and we spend time rewriting it. Are there alternate ways to work that give the best of both worlds?

from veering.

I understand why these functions main are there and that they might be useful in a development stage. Though, as part of a distributed library, I tend to believe that they create more confusion than help. If it is just for documentation purpose, it would make more sense to write documentation strings with specification and examples. The latter could automatically be tested with eg https://docs.python.org/3/library/doctest.html. This is how Python encourages users to write their library code.

I definitely support the idea of having code examples, notepads, code snippets, etc together with the code. A random person (including the ones who wrote the code) is more likely to copy/paste an example close from her purpose rather than reading function specifications. But I tend to separate such worked out examples in a disjoint folder such as scripts/ or examples/, or docs/. Such folder has no reason to be installed with the library.

from veering.

Ok, we will move the main() functions into separate documents named *_working.py, or *_examples.py

from veering.

I've started a "Scripts" directory, moved all of the "example code" to to it, and pushed.

from veering.

detail : Python tends to use snake case for files and folder (ie Veering -> veering, Scripts -> scripts)

from veering.

I did push a change which prepares for version 0.0

• I put all .py files that were tested in test_suite (+ dependencies) inside the veering/ folder that should be considered as the "veering library". All other files are in scripts. I believe that you want more files in veering. Simply tell me which one and what test should be added to the test suite.
• For testing, I strongly suggest to switch to pytest which is very popular these days
• The README file is adapted to the new setup

from veering.

I've used the new instructions in the README to install veering in sage.
I noticed that the module veering has no methods, which I thought was strange?
Anyway, I guessed that

from veering import test_suite

would work, and it does.
The test_suite completes correctly.
I also did a pass through the scripts.
I think that none of them are currently ready for prime time.

I will look at pytest. But I assume switching to pytest will be a big task?

from veering.

great that it works !

Indeed, one should import all relevant functions in the veering namespace. You need to choose what are the relevant functions and include them in the __init__.py as

from .module1 import functionX, objectY

Then the library could be used more conveniently as from veering import functionX (rather than from veering.module1 import functionX)

EDIT: it turns out that the examples from the README are broken because of this. taut_polynomial is not part of veering but veering.veering.

from veering.

Making the scripts work is I think the priority. For most of them, it is just a matter of changing the import statements.

Also, I believe that you would like some of the current scripts to be part of the veering library.

Switching to pytest is tedious but straightforward. I can do it.

EDIT: I put a sample test file in the test/ folder with some of the content of test_suite.py

from veering.

Ok, I am confused. Writing

"from .module1 import functionX, objectY"

suggests that we are going to allow functions of submodules to be functions of veering.

"You need to choose what are the relevant functions"

That feels like it violates the name space separation that we've evolved towards.
I was asking how the user is supposed to know the submodules of veering?

from veering.

Ok, I am confused. Writing

"from .module1 import functionX, objectY"

suggests that we are going to allow functions of submodules to be functions of veering.

Indeed.

"You need to choose what are the relevant functions"

That feels like it violates the name space separation that we've evolved towards. I was asking how the user is supposed to know the submodules of veering?

The submodules are usually about code organization. To my mind, it makes sense to have the main functions being available in the main namespace (here veering). Just take the very standard numpy

>>> import numpy
>>> numpy.array([0, 1, 2, 3])
array([0, 1, 2, 3])

If you dislike such approach, you can consider having the list of submodules in the (not yet existing) documentation.

One can also discover submodules via introspection but it is not very user friendly.

from veering.

Regarding scripts:

"For most of them, it is just a matter of changing the import statements."

I have fixed the import statements for all scripts, and checked that my scripts work. The scripts written by Henry or Anna will have to be checked by them (or by me, spending significantly more time...). I suggest that we ignore these for the moment.

"Also, I believe that you would like some of the current scripts to be part of the veering library."

There are a few that might be promoted, later. I suggest that we ignore these for the moment.

"Switching to pytest is tedious but straightforward. I can do it."

I have installed pytest, installed veering (both in sage and not in sage), fixed the tests in the test directory, and successfully run pytest at the command line in that directory. However, I do not know how to run pytest under sage... approximately 55% of the test suite needs sage.

from veering.

Oh, and I've checked that the test_suite still runs under sage (even after all the changes!). Whew.

from veering.

"Switching to pytest is tedious but straightforward. I can do it."

I have installed pytest, installed veering (both in sage and not in sage), fixed the tests in the test directory, and successfully run pytest at the command line in that directory. However, I do not know how to run pytest under sage... approximately 55% of the test suite needs sage.

To run pytest in the sage environment do

$ sage -sh
(sage-sh) $ pip install pytest # if not already done
(sage-sh) $ python -m pytest .

from veering.

Ok. In the sage shell:

python -m pytest .

throws an error as follows:

/Library/Frameworks/Python.framework/Versions/2.7/Resources/Python.app/Contents/MacOS/Python: No module named pytest

But simply typing

pytest

in sage shell, the test directory, now works. I had to uninstall veering and reinstall it (because the version was unchanged, I assume) for everything to work.

What should I do next? I suppose that you want all of the tests in the test suite moved and rewritten. Are you still willing to do that, or should I? Also, how should we deal with tests that require sage? Currently the test_suite pokes its environment a bit, and either does or doesn't do the test depending on the result. Having all of the tests in one place sort of makes sense to me - it is easier to use environmental flags, and it is easier to write new tests (by copying from old ones).

from veering.

Ok. In the sage shell:

python -m pytest .

throws an error as follows:

/Library/Frameworks/Python.framework/Versions/2.7/Resources/Python.app/Contents/MacOS/Python: No module named pytest

But simply typing

pytest

in sage shell, the test directory, now works. I had to uninstall veering and reinstall it (because the version was unchanged, I assume) for everything to work.

hmm, I think this is not calling the right pytest. Could you try which pytest? I mentioned in my earlier post that you have to install pytest inside the sage environment.

What should I do next? I suppose that you want all of the tests in the test suite moved and rewritten.

I find it cleaner with pytest but this is by no mean mandatory.

Are you still willing to do that, or should I?

I can definitely assist.

Also, how should we deal with tests that require sage?

There is a pytest directive importorskip for that purpose.

Currently the test_suite pokes its environment a bit, and either does or doesn't do the test depending on the result. Having all of the tests in one place sort of makes sense to me - it is easier to use environmental flags, and it is easier to write new tests (by copying from old ones).

from veering.

I believe that correctly installed pytest in sage, following your instructions. Here is a screen-shot for which pytest:

[~]$ sage -sh

Starting subshell with Sage environment variables set. Don't forget
to exit when you are done. Beware:

• Do not do anything with other copies of Sage on your system.
• Do not use this for installing Sage packages using "sage -i" or for
  running "make" at Sage's root directory. These should be done
  outside the Sage shell.

Bypassing shell configuration files...

Note: SAGE_ROOT=/Applications/SageMath
sage> which pytest
/Applications/SageMath/local/bin/pytest

from veering.

I find it cleaner with pytest but this is by no mean mandatory.

If it is not mandatory, let's skip it. We can fix the way we do testing in the future. Perhaps in version 1.61803398875.

from veering.

Dear Vincent - Is there anything left that really needs to be done? Can we close this issue?

from veering.

This looks like ready for a first beta release. Once the issue closed, I can proceed with making the release and uploading on PyPI. Unless you want me to teach you how to do that?

from veering.

Excellent. I am closing this issue. If you have time tomorrow at 3pm UK time, let's chat about teaching me this.

from veering.