Skip to content

Conversation

mwcraig
Copy link
Collaborator

@mwcraig mwcraig commented Sep 2, 2018

This pull request makes a number of changes to the way imports are done in vpython.

The intent is to not make any substantive changes to the names currently available to the user when they from vpython import *. Differences from current behavior are described below this overview.

Another significant change is moving RateKeeper2 and rate into rate_control.py.

All of the test programs (notebook and no-notebook) run with this with one exception: the notebook where you are supposed to be able to click on something and have it turn red doesn't work (it displays but clicking doesn't make it red).

The package still automatically decides whether to launch the notebook or no-notebook machinery.

These things need to be finished/decided before this is ready to merge:

  • Move initiation of notebook machinery out of __init__.py.
  • Remove creation of canvas from with_notebook.py (it is made in __init__.py now.
  • Tests must pass before merging
  • Decide: Should from vpython import * provide GSprint to the user? YES
  • Decide: Should from vpython import * provide MISC to the user? NO
  • Decide: Should from vpython import * provide RateKeeper to the user? NO
  • Decide: Should from vpython import * provide time to the user? NO, but clock should be
  • Restore clock to what comes with from vpython import *.

I don't have strong feelings about the "decides", I just don't know enough about what they do to know if users should see them by default.

List of names the user no longer gets with from vpython import *

Note that there are almost all either

  • Python standard libaries
  • Functions from standard libraries
  • Packages/variables specific to the no notebook case used internally by no_notebook.py.

If there are any of these that should be available it is easy to make them available.

Names no longer available:

  • BaseHTTPRequestHandler
  • GW
  • HTTPServer
  • INTERACT_PERIOD
  • MAX_RENDERS
  • MIN_RENDERS
  • USER_FRACTION
  • WebSocketServerFactory
  • WebSocketServerProtocol
  • WSserver
  • absolute_import
  • asyncio
  • copy
  • division
  • fd
  • find_free_port
  • glowcomm
  • httpserving
  • inspect
  • js
  • json
  • no_notebook
  • os
  • platform
  • print_function
  • serveHTTP
  • signal
  • signal_handler
  • signature
  • simulateDelay
  • socket
  • sys
  • threading
  • time
  • unquote
  • vpython
  • websocketserving

@mwcraig
Copy link
Collaborator Author

mwcraig commented Sep 2, 2018

Turns out that in the no-notebook case I completely removed MISC and GSprint. Didn't mean to...but are they necessary? Doesn't seem to have stopped the demos from working.

@BruceSherwood
Copy link
Owner

BruceSherwood commented Sep 2, 2018 via email

@mwcraig
Copy link
Collaborator Author

mwcraig commented Sep 3, 2018

Bruce -- I'll be pushing a couple other little changes in the next 30 min, but the instructions for trying out the branch are below. They the command line arguments you would use; some of it is almost certainly possible is SmartGit instead.

If you use Python environments you might want to make a new one for testing this, but that isn't necessary.

The instructions below assume you want to see this changes in the git folder you already have for VPython. If you want to make a clean copy, you could clone my version from here https://github.com/mwcraig/vpython-jupyter.git and then check out the branch refactor-imports. You would want to do that cloning in an empty directory to avoid clobbering what you already have.

In any event, the all-in-one-repo version:

# Get into the directory where your git repo of vpython-jupyter lives, and then..

# Let git know about my fork of the repo on github. You can use any name you want in place
# of mwcraig. I tend to name it for the github username of whoever's fork it is.
git remote add mwcraig https://github.com/mwcraig/vpython-jupyter.git

# Check that the remote was added (you will have at least two remotes, the one you just added
# and one likely called "origin" which is the one from which you originally cloned. You may have more.
git remote -v

# That only added the name of the remote. The next line pulls information about the names
# of branches, etc, from all of the remotes.
git fetch --all

# Now you want to check out my branch. If you used something other than "mwcraig" for
# the name of the remote above you will need to change the mwcraig below too.
git checkout -b refactor-imports mwcraig/refactor-imports

# Do a git status to make sure you are where you mean to be
git status

# Note that your local copy of the git repo now has a branch called refactor-imports
# If you just want to try my changes then you are done. As I add new commits you can
# pull them in to your local copy with 
git pull mwcraig refactor-imports

# IMPORTANT NOTE: If you want to make changes to this branch I would make a new branch
# from mine first. Checkout the branch as above, then make a new branch (use whatever name 
# for it you want)
git checkout -b refactor-imports-bruce

# Then, as you make changes and commit them you don't have to worry about my changes 
# getting in your way.

# To switch between the branches use (no -b)
git checkout branch_you_want

@mwcraig
Copy link
Collaborator Author

mwcraig commented Sep 3, 2018

Does MISC need to be exposed to the user too, or just GSprint?

@BruceSherwood
Copy link
Owner

The user doesn't need access to MISC; it's a helper class for GSprint; it makes possible connections to the JavaScript program. I'll mention that GSprint places text into html "body", below canvases and graphs. In the notebook case, this doesn't actually work; the text goes into the very end of the web page, not into the display area.

@mwcraig
Copy link
Collaborator Author

mwcraig commented Sep 3, 2018

Almost ready to push one more set of changes (last for today I hope). Some of the demos use time from the standard library. Should I add that back to the things import with from vpython import * or add import time to the demos?

@mwcraig
Copy link
Collaborator Author

mwcraig commented Sep 3, 2018

I am now done for the day if you want to test...

@BruceSherwood
Copy link
Owner

Concerning time, from the first page of the Help at glowscript.org:

As a convenience to novice programmers to provide everything that is needed to get started, VPython by default imports all of the VPython features and includes standard math functions such as sqrt. The documentation is written as though "from vpython import *" were present. Also provided are clock(), random(), and arange().

@mwcraig
Copy link
Collaborator Author

mwcraig commented Sep 4, 2018

Thanks for the link -- I've made a note in the "to do" list at the top of this to make sure clock and random are both present (I know arange already is).

@mwcraig
Copy link
Collaborator Author

mwcraig commented Sep 13, 2018

Oy. The travis stuff needs to be re-written a bit for the tests to run the way I want them to. Right now the tests always run in the latest version of python (3.7), which has a new function remainder in math, which surprises the tests. FIxing that on Thursday...

@mwcraig
Copy link
Collaborator Author

mwcraig commented Sep 13, 2018

I think I've fixed the tests that were failing. I've opened #61 and #63 to keep track of some travis issues that need fixing.

@mwcraig mwcraig changed the title [Work-in-progress] Change import structure to delay canvas render until necessary Change import structure to delay canvas render until necessary Oct 14, 2018
@mwcraig
Copy link
Collaborator Author

mwcraig commented Oct 15, 2018

:takes deep breath: -- merging this in!

@mwcraig mwcraig merged commit 9cb1402 into BruceSherwood:master Oct 15, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants