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

Zero to dev #1387

Merged
merged 22 commits into from Mar 8, 2022
Merged

Zero to dev #1387

merged 22 commits into from Mar 8, 2022

Conversation

bdevans
Copy link
Contributor

@bdevans bdevans commented Feb 14, 2022

These are the config files to build a development container automatically after pulling the repo and opening it in VS Code, hopefully lowering the barrier to entry.

Please have a go and let me know what you think @thesamovar @mstimberg !

@mstimberg
Copy link
Member

Looks cool! Unfortunately, I just realized that the remote development extension is not compatible with VS Codium (which I recently started using)... Apart from the non-free issue, I wonder whether explaining users how to install VS Code, the remote developing extension, and Docker, won't be more difficult than explaining how to set up a development environment in the "usual" way 😬

@bdevans
Copy link
Contributor Author

bdevans commented Feb 16, 2022

Thanks :) Ah pity about VS Codium (although I guess support might come later). As for setting up, I'd see it more of an alternative rather than a replacement for now. If someone has VS Code (quite likely) and Docker already installed, then cloning the repository will result in a pop-up offering to reopen the project in a container - one click and they're good to go. If they don't already have them and want to try this method, there are links to the VS Code (+ extension) and Docker installation instructions in the README which are pretty clear and easy to follow. Maybe just let several methods co-exist and see which naturally has the most uptake after a while?

@mstimberg
Copy link
Member

No, sure, I wasn't suggesting to not include it (even though I'd like to test it first ;-) ). For the pop-up to appear, what exactly do you need? Only VS Code would be great (if it tells you then to install the rest), but IIUC it is VS Code + Remote extension + Docker, right?

Ah pity about VS Codium (although I guess support might come later)

I think it's not a technical issue, but rather one of licencing:

Portions of the Remote Development extensions are used in developer services that are run under a proprietary license. While these extensions do not require these services to work, there is enough code reuse that the extensions are also under a proprietary license. While the bulk of the code is in the extensions and in the Code - OSS repository, a handful of small changes are in the Visual Studio Code distribution. wiki

@bdevans
Copy link
Contributor Author

bdevans commented Feb 16, 2022

Ok cool. Well since you don't have VS Code installed currently, you could be a great guinea pig ;) If you install VS Code first then clone the repo (the landing page has an option to "Clone Git Repository...") see if it recognises the .devcontainer folder and prompts you to install the other bits. If not, install the remote development extension and see if it prompts you to install Docker (if not, do so manually) and then you should get the pop-ups guiding you through. If you note the start and end times, the time needed to get up and running could be a useful bit of info to include in the README.

@mstimberg
Copy link
Member

mstimberg commented Feb 16, 2022

Hah, that actually went quite smoothly. With a fresh VS Code install, it indeed asked me to install the remote development extensions and then asked to reload the repo. This triggered a request to install docker, which I duly did. This was the only time when it wasn't as smooth as it could be: I had to trigger the "reload in container" command manually after the docker installation. Then, it started building the container (took a bit over 2 mins on my slow-ish machine with fast internet), and finally it asked for another refresh for pylance. And then I can run stuff via the container, pretty cool!
One thing that does not work is plotting (it uses the Agg backend I think, i.e. it can only plot to files not display). I had a quick search and I could possibly fix this by forwarding a bunch of environment variables and an XAuthority file, but seems non-trivial.

@bdevans
Copy link
Contributor Author

bdevans commented Feb 16, 2022

Glad to hear it went smoothly! :)

Yes, plotting needs a bit of thought. I am generally happy to plot to file and view the png within VS Code or a file browser but I appreciate not everyone will be. I think the easiest solution (rather than needing an X server to forward to) might be to install JupyterLab and encourage people to plot within that.

@bdevans
Copy link
Contributor Author

bdevans commented Feb 18, 2022

What do you think? Is it worth including Jupyter in the container or should we just make do with plots written directly to file?

@mstimberg
Copy link
Member

Sorry about being slow with this, I've been quite busy with other things...

Is it worth including Jupyter in the container or should we just make do with plots written directly to file?

Would this run the jupyter server in the container and the jupyter interface in Visual Code? If yes, and if it does not need much manual configuration for the user, this would be great. If not, and/or if it needs a lot of manual setup, then it won't probably make things much easier for users.

But in general, since the main idea for the docker container is to make it easier for developers, I don't think having easy access to plots is that important.

@bdevans
Copy link
Contributor Author

bdevans commented Feb 22, 2022

Would this run the jupyter server in the container and the jupyter interface in Visual Code? If yes, and if it does not need much manual configuration for the user, this would be great. If not, and/or if it needs a lot of manual setup, then it won't probably make things much easier for users.

Yes (at the cost of a little extra bulk and slightly longer build times but not excessive). I've also added ipympl so that you can use %matplotlib widget in the notebooks for resizing, panning and zooming etc. I've added some settings which hopefully means this needs no further configuration to work out of the box container! :)

@bdevans
Copy link
Contributor Author

bdevans commented Feb 25, 2022

Hi Marcel, the container has JupyterLab setup now so that you can do interactive plotting in notebooks all within the container, so I think it's ready to merge now.

@mstimberg
Copy link
Member

I just tried out the jupyter integration, and I could run a notebook directly in the VS Code setup/container. Great!

We should probably make the documentation a bit more discoverable? Not sure whether a README in the .devcontainer is the best place, maybe we should move it to the developer docs? Or maybe leave it where it is, but simply mention it in the developer docs.

What I am not 100% sure about for now is the dev-requirements.txt file. Do I understand it correctly that the advantage of installing the dependencies as part of the Dockerfile (instead of in the postCreateCommand) is that we can upload a docker image with the dependencies and users will be able to skip the whole building step? Or does it also make a difference on your own computer? I'm a bit cautious about introducing a new place where we have to keep dependencies up to date (the system is already a bit of a mess, but I was waiting for a general switch to the PEP517/PEP518 mechanism). If we keep dev-requirements.txt, then it also needs to include cython and sympy, and (I think) sphinx-tabs to build the documentation. Brian also depends on pyparsing and jinja2, but it seems they are already installed as indirect dependencies.

We should also think about testing all this on our CI infrastructure, but maybe let's postpone this for now (would be good to not have this make our default test suite take even longer...).

@bdevans
Copy link
Contributor Author

bdevans commented Feb 28, 2022

Glad it worked! :)

Sure, I can point to that README in the developer docs. Should I just put a sentence or two about 'an easy way to get started' in docs_sphinx/developer/index.rst or does it warrant its own page?

The dev-requirements.txt is just a way of specifying the extra dependencies that a developer will likely need (which don't get automatically installed with Brian). I don't think there is an automatic way of installing these currently and just saw them listed on the README, so thought it would be worth making a pip-installable list.

Yes, we could publish a Docker brian2/developer image with the dependencies setup to reduce build time but a fresh build doesn't take too long anyway and initially my thinking was that putting them in an earlier layer of the build process would speed up rebuild times as these dependencies are likely to change less frequently than core dependencies (installed as part of the postCreateCommand), which will potentially be updated as part of a PR during development as bugs are found or new features introduced. E.g. the numpy minimum version gets bumped in order to use a new feature in code under development but this way when setup.py is updated, SciPy, Matplotlib etc. don't need to be reinstalled when the container is rebuilt, speeding up the process.

I'll add those extra dependencies to dev-requirements.txt for now then until a better mechanism is available. I'm happy to revisit that once there are ways to centralise the two sets of dependencies.

Might be worth adding this to the testing infrastructure (if possible) since it doesn't take too long but that could wait a bit if preferred.

@mstimberg
Copy link
Member

Sure, I can point to that README in the developer docs. Should I just put a sentence or two about 'an easy way to get started' in docs_sphinx/developer/index.rst or does it warrant its own page?

I'd be happy with a simple pointer for now. Maybe we can have a full page when we propose a docker image as a more general solution (i.e. not only in the context of VS Code)

The dev-requirements.txt is just a way of specifying the extra dependencies that a developer will likely need (which don't get automatically installed with Brian). I don't think there is an automatic way of installing these currently and just saw them listed on the README, so thought it would be worth making a pip-installable list.

For installing the developer dependencies, you can use the [test] and [doc] extra requirements defined in setup.py. This means that the postCreateCommand could use pip3 install --user -e '.[test,doc]' to install everything necessary.

E.g. the numpy minimum version gets bumped in order to use a new feature in code under development but this way when setup.py is updated, SciPy, Matplotlib etc. don't need to be reinstalled when the container is rebuilt, speeding up the process.

I think I get this in principle: installing matplotlib and jupyter in the container would mean that re-installing Brian after changes in setup.py would be quicker (this wouldn't really be a "rebuild" of the container, would it? The container wouldn't actually change). I have to say I'm no longer particularly worried about installation times, though. The days when scipy had to be compiled from scratch are luckily over, installing it via pip nowadays is a matter of seconds.

@bdevans
Copy link
Contributor Author

bdevans commented Mar 2, 2022

I'd be happy with a simple pointer for now. Maybe we can have a full page when we propose a docker image as a more general solution (i.e. not only in the context of VS Code)

Ok I've added a paragraph with some links to the .devcontainer documentation.

For installing the developer dependencies, you can use the [test] and [doc] extra requirements defined in setup.py. This means that the postCreateCommand could use pip3 install --user -e '.[test,doc]' to install everything necessary.

I've removed all the dependencies that wouldn't be installed with pip3 install --user -e '.[test,doc]' from dev-requirements.txt now, which just leaves scipy, matplotlib and jupyterlab.

I think I get this in principle: installing matplotlib and jupyter in the container would mean that re-installing Brian after changes in setup.py would be quicker (this wouldn't really be a "rebuild" of the container, would it? The container wouldn't actually change). I have to say I'm no longer particularly worried about installation times, though. The days when scipy had to be compiled from scratch are luckily over, installing it via pip nowadays is a matter of seconds.

The image would need to be rebuilt if the new dependency version were not already satisfied but I guess this would be rare enough not to worry about.

.devcontainer/README.md Outdated Show resolved Hide resolved
.devcontainer/README.md Outdated Show resolved Hide resolved
@bdevans
Copy link
Contributor Author

bdevans commented Mar 7, 2022

Hey Marcel, are there any other changes you'd like me to make?

@mstimberg
Copy link
Member

Hey Marcel, are there any other changes you'd like me to make?

Nope, looks great to me!

@mstimberg mstimberg merged commit 162e60e into master Mar 8, 2022
@mstimberg mstimberg deleted the zero_to_dev branch March 8, 2022 10:37
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.

None yet

2 participants