-
Notifications
You must be signed in to change notification settings - Fork 100
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
Cleaner install #141
Cleaner install #141
Conversation
Instead we specify the path to each subpackage in the calls to distutils.
@mandli @rjleveque I'd like to discuss this and decide on merging it within the next week or so, so we don't forget about it. Let me know if you don't have time to review it in that timeframe. I believe that there is nothing backward-incompatible here. It would be wise when people update for them to remove the symlinks from their old installation, or just create a new install. But as far as I can tell, leaving the old symlinks in place won't actually break anything. |
@ketch This all makes a lot of sense to me but I have not had a chance yet to exhaustively test it. |
I tried this and got as far as...
|
@rjleveque Hmmmm, I tested this again in a virtualenv and couldn't reproduce that. Also, the instructions up to that point are essentially identical to those of #139, which didn't give you this error. Could you double-check that you're typing everything exactly (or just copy-paste it)? Has anyone else had this problem? |
I tried this and it seemed to work for me. |
I tried this again from scratch and got the same error. So I added a remote But the
|
I'm glad that fixed it. I don't understand this since I've done this in a virtualenv with a fresh clone, where nothing was set up except the instructions above. In any case, this won't be an issue once this is merged.
I'm having a hard time imagining any way this could be related to the PR (we certainly don't mess with the submodule structure here). Maybe a flaky internet connection that timed out? What happens if you rerun |
No change to the empty visclaw directory when I redo the submodule init/update. I also tried from scratch in a new conda environment with the same behavior. I'll send you the output. |
@rjleveque could you try this:
|
Strange, but after doing
the ketch remote in pyclaw already exists but points to the wrong repository (as does origin, I think):
Shouldn't these point to Ditto in riemann,
|
@rjleveque I have no idea why that would happen. It's annoying that these issues have nothing to do with this PR and seem to be problems with navigating submodules and multiple github forks. I have pushed all three branches (in clawpack, pyclaw, and riemann) to the main repos so now you shouldn't need to add the ketch remote at all. Please try:
|
So now these commands work in the sense that no errors are generated. But the output below has me a bit mystified... Why does it install to
|
@rjleveque I haven't used conda environments much, but I don't think this is anything special about Clawpack. If you do an editable install of, say, numpy, where does it put things? I'm guessing it's an issue with your conda environment setup, but let me know if I'm wrong. I'm using virtualenv and this installation puts things only in the site-packages specific to my virtualenv. |
Sorry for not getting back to this sooner. I think it's fine to merge but I'll let @mandli do it since there are now merge conflicts with clawpack/pyclaw#612. I just tested this PR on an AWS EC2 instance to avoid some of the issues on my laptop. It worked fine, after installing git, numpy, etc. The one thing I had to change was the pip install command to
This is something we should mention in general in the docs for people installing on any machine they don't have root access to (although on AWS I probably could have also done Before long we should update the I can add the full set of installation instructions for getting things running on AWS too. All the unit tests ran in all repositories except the problem I raised in clawpack/riemann#146 that is unrelated to this PR. |
I have used |
Hooray! Suggesting that people use
I think the only change to the instructions is that there is no longer a |
That looks right to me. |
I don't understand why there are still conflicts here. I merged the recent commits to pyclaw and riemann (for the HLLE solvers) into these branches. I don't know what the conflicts are or how to "resolve" them. |
I did the following on my machine:
and I got
So I'm really puzzled as to why github thinks there are conflicts. Should I just push to master? |
I get the same, and gitk doesn't show any potential conflicts. So yes, seems ok to me to push back to master. |
This replaces #140. This version does not move the fortran code in riemann, so it does not require any changes to amrclaw, geoclaw, or classic. It does have all the benefits of #140.
To test, do the following: