Skip to content
This repository was archived by the owner on Nov 29, 2019. It is now read-only.

Switch to pyctdev #61

Merged
merged 24 commits into from
Jul 8, 2018
Merged

Switch to pyctdev #61

merged 24 commits into from
Jul 8, 2018

Conversation

ceball
Copy link
Member

@ceball ceball commented Jun 9, 2018

  • packaging metadata declared in setup.cfg (including run time dependencies) (*1)
  • automatic versioning (uses autover via param)
  • build time dependencies declared in pyproject.toml
  • conda build is templated (avoids duplicating metadata) except for build time dependencies (doesn't support pyproject.toml)
  • user command (parambokeh examples) for copying examples (using pyct.cmd)
  • clean up packaging and testing (adds pypi+wheel, adds package testing; all test commands etc are described in tox.ini)

Note that various extra metadata is now supported by pypi (below from a test parambokeh package on testpypi):
screen shot 2018-06-11 at 22 27 51

(*1) using declarative setup.cfg is optional; could just as well use setup.py. (Except for license_file for wheel, which must be in setup.cfg - hence setup.cfg must exist...already received at least 5 PRs recently adding it to pyviz projects...) What do you prefer?

(*2) [out of date/ignore] Alternatively could also make autover packages and then add as an extra build dependency. Or alternatively again, could add param as an extra build dependency. What do you prefer? Whichever we do, note that I've made a temporary "get_setup_version2" function (used in setup.cfg) that can read reponame (and pkgname if different) from setup.cfg, e.g.

[tool:autover]
reponame = parambokeh

(*3) Although I cut down the code involved in packaging the examples, it still puts two copies of the examples into the package and will copy example data for a developer install. Reworking the examples packaging is future work (holoviz-dev/pyct#22; hopefully can fix just once in pyctbuild and then will be ok everywhere).

@ceball ceball requested a review from jbednar June 11, 2018 22:10
@ceball
Copy link
Member Author

ceball commented Jun 12, 2018

https://github.com/pyviz/pyctdev/blob/master/instructions.md might be useful for an overview of the files involved.

@ceball
Copy link
Member Author

ceball commented Jun 12, 2018

(the "not yet ready to merge" comment in the description is just about the tests - e.g. flake check with python 2.7 has picked up a py27 syntax error - rather than that the main content of the PR isn't finished)

@ceball ceball force-pushed the switch_to_pyctdev branch from 9c7c9d1 to 3c2ca01 Compare June 13, 2018 08:03
Copy link
Member

@jbednar jbednar left a comment

Choose a reason for hiding this comment

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

Thanks. There seem to be several outstanding issues, but it seems like an improvement...

.travis.yml Outdated
script:
- doit develop_install $CHANS_DEV -o doc -c conda-forge # phantomjs still not on defaults
Copy link
Member

Choose a reason for hiding this comment

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

What does parambokeh need phantomjs for?

setup.cfg Outdated
author_email = [email protected]
maintainer = PyViz
maintainer_email = [email protected]
url = https://ioam.github.io/parambokeh
Copy link
Member

Choose a reason for hiding this comment

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

I don't quite understand why projects are never seeming to move from ioam to pyviz; is there really any reason to create more references to the old name? Why not just move them immediately?


doc =
nbsite
sphinx_ioam_theme
Copy link
Member

Choose a reason for hiding this comment

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

Same here; why are we continuing to use sphinx_ioam_theme? Shouldn't this be a new name, at least, like pyviz_theme?

setup.py Outdated
packages = ["parambokeh"],
provides = ["parambokeh"],
))
raise ImportError("Parambokeh requires pyctbuild to build; please upgrade to pip>=10 and try again (or alternatively, install pyctbuild manually first (e.g. `conda install -c pyviz pyctbuild`)")
Copy link
Member

Choose a reason for hiding this comment

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

And maybe add param to what's proposed to be installed here, just in case (for autover)?

setup.py Outdated
if 'develop' not in sys.argv:
import pyctbuild.examples
pyctbuild.examples.examples(example_path, __file__, force=True)

Copy link
Member

Choose a reason for hiding this comment

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

In the description you mention that you hope the double-copying of examples can be fixed once at the top level, but it's hard to see how, if you're copying the examples here in each project.

setup.py Outdated
setup()

if os.path.isdir(example_path):
shutil.rmtree(example_path)
Copy link
Member

Choose a reason for hiding this comment

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

This seems like it could delete the contents of my examples directory, which would be very dangerous. Right?

@jbednar
Copy link
Member

jbednar commented Jun 13, 2018

What do you prefer?

Can you generate setup.cfg from the files you already have? We do need the license files to appear in wheels, but it seems a pain to maintain a file solely for that purpose, if it could be generated as needed.

@ceball ceball force-pushed the switch_to_pyctdev branch from 20f6b8b to 5e42262 Compare July 7, 2018 18:53
@ceball ceball changed the title (WIP) Switch to pyctdev Switch to pyctdev Jul 7, 2018
Copy link
Member

@jbednar jbednar left a comment

Choose a reason for hiding this comment

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

Looks good; please merge.

setup.cfg Outdated
url = https://ioam.github.io/parambokeh
project_urls =
Bug Tracker = https://github.com/ioam/parambokeh/issues
Documentation = https://ioam.github.io/parambokeh
Copy link
Member

Choose a reason for hiding this comment

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

Not parambokeh.pyviz.org?

Copy link
Member Author

Choose a reason for hiding this comment

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

I could change this. I also could change it in doc/conf to be the canonical url (one day, doc/conf will get various info from elsewhere to avoid duplication). However, I personally don't understand the consequences of not using https (search rankings, user warnings - e.g. https://security.googleblog.com/2018/02/a-secure-web-is-here-to-stay.html). So to confirm: I should still change it?

Copy link
Member

Choose a reason for hiding this comment

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

Is there a reason not to use https://parambokeh.pyviz.org/?

Copy link
Member Author

Choose a reason for hiding this comment

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

No - great (I'd just typed in parambokeh.pyviz.org and received the 'not secure' note). So I'll change this.

Copy link
Member Author

Choose a reason for hiding this comment

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

@philippjfr, do you happen to know why the same isn't true of pyviz.org? I.e. while https://parambokeh.pyviz.org works, https://pyviz.org doesn't seem to. I've lost track of who's doing what/who owns which domain...

@ceball ceball merged commit be39791 into master Jul 8, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants