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.

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

Python 3: Package structure and relative imports #193

Closed
dgelessus opened this issue Jul 12, 2016 · 2 comments · Fixed by #314
Closed

Python 3: Package structure and relative imports #193

dgelessus opened this issue Jul 12, 2016 · 2 comments · Fixed by #314

Comments

@dgelessus
Copy link
Contributor

I've been looking at Stash's package structure a bit (for the universal branch). I noticed that many modules use implicit relative imports (e. g. stash.py imports system.shcommon, which actually imports stash.system.shcommon) which do not work under Python 3. This is not a big issue, most of these can be replaced with real relative imports (e. g. from .system import shcommon).

The second issue (not directly related to the first one) is that the repo root is the stash package folder, i. e. site-package/stash is the repo root. This means that you have to clone the repo into site-packages/stash, or some imports (for example in launch_stash.py will fail). So it's not possible to have two versions of Stash installed without changing the package name and adjusting imports.

A solution to this would be to move all Stash module files (like stash.py and system) into a stash subfolder, while leaving non-module files in the repo root (like launch_stash.py). This would make installing multiple versions or switching between branches much easier. The launch_stash.py would be in the same folder as the stash package, so when it imports stash it automatically uses the version next to it instead of the one from site-packages.

For normal users nothing would change - if there is no stash package in the same folder as launch_stash.py, it looks through the other module locations, including site-packages. getstash.py would only need some minor modifications to copy the stash folder into site-packages and not the entire repo.

@ywangd
Copy link
Owner

ywangd commented Jul 18, 2016

Yes the relative imports need to be fixed. Thanks.

However, having multiple stash installations is actually something I try to avoid. I agree it is useful for development purpose. But for most users, this possibility is more likely to cause confusions, e.g. one may wonder why selfupdate does not work as it is probably updating another version which is not the currently running one.

@dgelessus
Copy link
Contributor Author

Yes, that's understandable, users shouldn't install multiple stash instances by accident. However I don't think this change would affect stash installations in site-packages - as long as we change getstash.py to understand the new structure, selfupdate would work the same as before for normal users.

The difference is that with the old structure, when you clone the repo by hand into a different folder, the launch_stash.py in the repo would still use the stash installation in site-packages, and not the one in the cloned repo. With the new structure, launch_stash.py would prefer importing stash from the current directory, and fall back to site-packages if there is no valid stash package next to the launch_stash.py.

bennr01 added a commit to bennr01/stash that referenced this issue May 4, 2018
#About this commit/PR
This is a commit/PR to close a large number of old/fixed issues at once using the github `fix #<issuenumber>` syntax.
Below is a list of all issues which will automatically be closed by merging this commit/PR into `ywangd/master`.
Behind each issue number is a short summary about the issue or why it can be closed.

#Issues which can be closed

**Py3 related:**
- fix ywangd#281 (py3 had not yet been supported)
- fix ywangd#261 (python3 pip installation)
- fix ywangd#246 (python3 code in python2)
- fix ywangd#222 (python3 command in stash)
- fix ywangd#200 (pip now works (at least partially) with python3)
- close ywangd#197 (general pythonista3 compatibility thread)


**Questions which have been answered / unfixable bugs:**
- close ywangd#310 (pip installation with c code)
- close ywangd#292 (StaSh is not dead; also a question regarding py3)
- close ywangd#290 (pip installation, problem solved)
- close ywangd#289 (answer provided, no further activity by reporter)
- close ywangd#279 (pip install protects standard distribution)
- close ywangd#245 (answered)
- close ywangd#244 (not possible (unless someone finds a workaround)
- close ywangd#243 (c-code in pip installation)
- close ywangd#208 (question answered)


**implemented suggestions/changes:**
- fix ywangd#273 (partially implemented as part of py2and3)
- fix ywangd#193 (implemented by now)
- fix ywangd#63  (subcommand completion; implemented long ago)
- fix ywangd#39  (pip; implemented long ago)

**fixed bugs:**
- fix ywangd#308 (https error in pip; fixed)
- fix ywangd#223 (old beta-related bug)
- fix ywangd#211 (old beta-related bug)
- fix ywangd#209 (seems to be fixed, can not reproduce)
- fix ywangd#206 (git did not clone into subdirectory; fixed)
- fix ywangd#98  (telnet did not send \r\n; fixed)


**other stuff:**
- close ywangd#260 (no useful bug description)
- close ywangd#258 (no useful bug description)
- close ywangd#251 (not a bug but an announcement, no longer relevant)
- close ywangd#241 (works now)
- close ywangd#219 (no further acivity by reporter, likely user error)


#Issues which can be closed, but where the underlying issue has not been fixed
- close ywangd#285 (pip command too complex)
- close ywangd#291 (pip installation question, but not c-code related. But seems package-related)
- close ywangd#274 (pip install problem, at least partial related to c-code, no further activity by reporter)
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 a pull request may close this issue.

2 participants