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

[IMPROV] Quality of Life improvments for the devcontainer #163

Merged
merged 10 commits into from
Jun 19, 2024

Conversation

AlexVCaron
Copy link
Collaborator

@AlexVCaron AlexVCaron commented Jun 18, 2024

Type of improvement

If submitting a new module or fixing a bug, please use the appropriate template.

  • Documentation
  • Development tools (e.g. linter, formatter, etc.)
  • Development container
  • Global update (please specify)
  • Other (please specify)

Describe your improvement

Quality of life improvements for the development container :

  • Auto-populate NFCORE_*_GIT_REMOTE and NFCORE_*_BRANCH. No need to add --git-remote or --branch to the nf-core commands inside the container anymore
  • Reinstate the .venv python environment inside the workspace. We'll ignore it everywhere we can, but it must live there for vscode to detect it correctly.
  • Add user profile persistence (command history should carry between rebuilds). Requires to rebuild the container once, so the volumes get instantiated and linked to the container by docker
  • Place the .nf-test folder living inside the workspace in its own volume, so we can delete it easily if the container runs out of space and does want to start
  • Add NiiVue to the extensions stack. Enjoy looking at images from inside vscode now ! Now, when you click on a nifti in the explorer, it opens in a 3D (even 4D !) viewer in the editor. The 3D mean intensity projection view is quite nice, ! But it does handle all space transformations perfectly yet, doesn't have peaks, nor fodf, nor tensors and is limited in sense of colormaps. Still, very nice to have in the container.
  • Split devcontainer external configuration (not much, but it's clearer what the content of the .devcontainer folder does)

Describe how to test your improvement

Remove all containers and volumes related to the nf-scil devcontainer from docker. All resources should be prefixed with nf-scil. You won't loose your work, but you'll loose any cache living outside the workspace folder.

Checklist before requesting a review

  • Ensure the syntax is correct (EditorConfig and Prettier must pass)
  • Run the test suites if your changes affect any module
  • Regenerate the Poetry lock file if you have updated the dependencies
  • Ensure the documentation is up-to-date

@AlexVCaron
Copy link
Collaborator Author

Still need to update all documentation 😜

@AlexVCaron AlexVCaron changed the title Feat/nf core remote [IMPROV] Add Quality of Life improvment to the devcontainer Jun 18, 2024
@AlexVCaron AlexVCaron changed the title [IMPROV] Add Quality of Life improvment to the devcontainer [IMPROV] Quality of Life improvments for the devcontainer Jun 18, 2024
@AlexVCaron
Copy link
Collaborator Author

Documentation should be updated !

Copy link
Contributor

@gagnonanthony gagnonanthony left a comment

Choose a reason for hiding this comment

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

Wow, that's really nice! 💯 I just tested it, and it truly makes it much easier to develop within the container (even on Mac...). I encountered only one bug, which I described below. Otherwise, it looks pretty good to me!


ignore [".venv/*"]
Copy link
Contributor

Choose a reason for hiding this comment

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

It seems the ignore flag is not available in the current release of nf-test (0.8.4). When I try to run a test for a module, it outputs this error message:

Error: Syntax errors in nf-test config file: groovy.lang.MissingPropertyException: No such property: ignore for class: com.askimed.nf.test.config.Config

I quickly went through the nf-test repo, and it seems the ignore was commited last month for the 0.9.0 release. I also might be wrong 😆

Copy link
Contributor

@arnaudbore arnaudbore left a comment

Choose a reason for hiding this comment

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

GTG

@arnaudbore arnaudbore merged commit defffee into main Jun 19, 2024
7 checks passed
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

3 participants