Skip to content

Improve Development Accesibility - Devcontainer [part 1 / 2] #41721

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.

Sign up for GitHub

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

Closed
wants to merge 2 commits into from
Closed

Improve Development Accesibility - Devcontainer [part 1 / 2] #41721

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
e690a4e
make devcontainer stateful
cgarciae May 27, 2021
8ea30ab
rename container_start.sh => container_create.sh
cgarciae May 29, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 7 additions & 5 deletions .devcontainer.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,13 +4,13 @@
"name": "pandas",
"context": ".",
"dockerFile": "Dockerfile",

"postCreateCommand": "bash container_create.sh",
// Use 'settings' to set *default* container specific settings.json values on container create.
// You can edit these settings after create using File > Preferences > Settings > Remote.
"settings": {
"terminal.integrated.shell.linux": "/bin/bash",
"terminal.integrated.defaultProfile.linux": "/bin/bash",
"python.condaPath": "/opt/conda/bin/conda",
"python.pythonPath": "/opt/conda/bin/python",
"python.pythonPath": "/workspaces/pandas/.venv/bin/python",
"python.formatting.provider": "black",
"python.linting.enabled": true,
"python.linting.flake8Enabled": true,
Expand All @@ -21,10 +21,12 @@
"pandas"
]
},

// Add the IDs of extensions you want installed when the container is created in the array below.
"extensions": [
"ms-python.python",
"ms-vscode.cpptools"
],
"runArgs": [
"--privileged"
]
}
}
3 changes: 3 additions & 0 deletions .gitignore
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -119,3 +119,6 @@ doc/build/html/index.html
doc/tmp.sv
env/
doc/source/savefig/

# container
.venv
25 changes: 0 additions & 25 deletions Dockerfile
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,29 +20,4 @@ RUN apt-get update \
&& apt-get clean -y \
&& rm -rf /var/lib/apt/lists/*

# Switch back to dialog for any ad-hoc use of apt-get
ENV DEBIAN_FRONTEND=dialog

# Clone pandas repo
RUN mkdir "$pandas_home" \
&& git clone "https://github.com/$gh_username/pandas.git" "$pandas_home" \
&& cd "$pandas_home" \
&& git remote add upstream "https://github.com/pandas-dev/pandas.git" \
&& git pull upstream master

# Because it is surprisingly difficult to activate a conda environment inside a DockerFile
# (from personal experience and per https://github.com/ContinuumIO/docker-images/issues/89),
# we just update the base/root one from the 'environment.yml' file instead of creating a new one.
#
# Set up environment
RUN conda install -y mamba
RUN mamba env update -n base -f "$pandas_home/environment.yml"

# Build C extensions and pandas
SHELL ["/bin/bash", "-c"]
RUN . /opt/conda/etc/profile.d/conda.sh \
&& conda activate base \
&& cd "$pandas_home" \
&& export \
&& python setup.py build_ext -j 4 \
&& python -m pip install -e .
12 changes: 12 additions & 0 deletions container_create.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
# Switch back to dialog for any ad-hoc use of apt-get
Copy link
Member

Choose a reason for hiding this comment

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

-1 on adding this file. I think ideally we shouldn't even have vscode stuff in the repo, and we should be agnostic of what editor or IDE devs are using in this repo. I understand that it can be convenient to have a settings file for vscode, and we already do, but adding more vscode scripts to the repo seems excesive. If ci/setup_env.sh can't be used, and this code can't be added directly to the vscode config file, maybe it's a good time to create a separate project for vscode stuff for pandas development.

Copy link
Member

Choose a reason for hiding this comment

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

Yea I agree with Mark here. I think adding stuff specific to an IDE is favoring a workflow that we should probably be agnostic to. Maybe better served as a dedicated pandas-vscode repo that just uses pandas as a submodule?

Copy link
Author

@cgarciae cgarciae Aug 5, 2021 •
edited
Loading

Choose a reason for hiding this comment

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

Hmmm. The .devcontainer.json is already an editor-specific file so that line has already been crossed.

How about we create a /.devcontainer folder and move both the .devcontianer.json and container_create.sh files into it? This way we don't have these files pollute the workspace.

export DEBIAN_FRONTEND=dialog

# Set up environment
conda install -y mamba
mamba env update -p .venv -f "environment.yml"

source activate /workspaces/pandas/.venv

# build + install pandas
python setup.py build_ext -j 4
python -m pip install -e .
Copy link
Contributor

Choose a reason for hiding this comment

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

add a final eol