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

Refactor devcontainer 📦 #329

Open
wants to merge 9 commits into
base: develop
Choose a base branch
from
Open

Conversation

sduquemesa
Copy link
Contributor

@sduquemesa sduquemesa commented Jan 26, 2024

Context:
Doing some housekeeping here on the devcontainer side.

Description of the Change:

  • The previous build script was a bit cumbersome, here it has been simplified.
  • Julia installation is now manually managed with juliacall, meaning there is no need for passing the optional JULIA_INSTALL arg to docker build.
  • Only a single Dockerfile to create the container. This Dockerfile can also be used to create a completely isolated environment using docker (useful for testing, running simulations, etc.).
  • Ensures changes on the code are reflected on the interpreter as they happen. To do so the workspace folder points to /mrmustard where the source code resides.
  • Installs the jupyter extension automatically.
  • Users can now have the MRMUSTARD_PYTHON_VERSION env-var set in their local environment to dictate which python version the dev container will be built with. When unset, the default value is 3.10. I'd suggest doing this in your bashrc/zshrc/whatever-rc to ensure it's always set and therefore picked up by VS Code

Benefits:

  • Click and go 🚀
  • Seamless julia installation
  • Customizable python version

Possible Drawbacks:
None

@sduquemesa sduquemesa added the no changelog Pull request does not require a CHANGELOG entry label Jan 26, 2024
{
"name": "Mr Mustard",
"name": "MrMustard 🌭",
Copy link
Collaborator

Choose a reason for hiding this comment

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

haha love it

env.Dockerfile Outdated
@@ -0,0 +1,40 @@
# *** Base *** #
FROM python:3.10 AS base
Copy link
Contributor

Choose a reason for hiding this comment

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

Were we always using 3.10 for docker, or 3.9?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

We had 3.9 to match the lowest python version supported by MrMustard but @ziofil uses 3.10 more often inside devcontainers

Copy link
Collaborator

Choose a reason for hiding this comment

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

If it were for me I'd use 3.12! Maybe now 3.11 is supported too

Copy link
Collaborator

Choose a reason for hiding this comment

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

I have updated the devcontainer definition so it will build with 3.10 by default, but you can specify another version if you please. To do this, just have the MRMUSTARD_PYTHON_VERSION environment variable set locally before building+opening the devcontainer (default will be 3.10).

Some notes:

  • I feel like VS Code always refreshes something before building, so I'd put it in your ~/.bashrc, ~/.zshrc` or whatever you use to be safe
  • We can't build with 3.12 because tensorflow fails to install 🙃

@sduquemesa
Copy link
Contributor Author

@ziofil, I've readded oh-my-zsh to the container as default terminal

Copy link

codecov bot commented May 29, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 87.61%. Comparing base (c16f798) to head (3e9af05).

Additional details and impacted files
@@           Coverage Diff            @@
##           develop     #329   +/-   ##
========================================
  Coverage    87.61%   87.61%           
========================================
  Files           81       81           
  Lines         6154     6154           
========================================
  Hits          5392     5392           
  Misses         762      762           

Continue to review full report in Codecov by Sentry.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update c16f798...3e9af05. Read the comment docs.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
no changelog Pull request does not require a CHANGELOG entry
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants