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

VScode settings #128

Closed
6 tasks done
jan-matthis opened this issue Apr 16, 2020 · 21 comments · Fixed by #148
Closed
6 tasks done

VScode settings #128

jan-matthis opened this issue Apr 16, 2020 · 21 comments · Fixed by #148
Assignees
Labels
enhancement New feature or request good first issue Good for newcomers help wanted Extra attention is needed

Comments

@jan-matthis
Copy link
Contributor

jan-matthis commented Apr 16, 2020

Initial inspiration:

{
    //
    // Editor settings for python
    //
    "[python]": {
        "editor.defaultFormatter": "ms-python.python",
        "editor.formatOnSave": true,
        "editor.codeActionsOnSave": {
            "source.organizeImports": true
        }
    },
    //
    // Formatting
    // https://code.visualstudio.com/docs/python/editing#_formatting
    //
    "python.formatting.provider": "black",
    "python.formatting.blackPath": "/Users/jm/opt/anaconda3/envs/sbi/bin/black",
    //
    // Sort imports
    // https://github.com/microsoft/vscode-python/issues/5840#issuecomment-497321419
    //
    "python.sortImports.path": "/Users/jm/opt/anaconda3/envs/sbi/bin/isort",
    "python.sortImports.args": [
        "--settings-path=/Users/jm/sbi/setup.cfg"
    ],
    //
    // Linting
    // https://code.visualstudio.com/docs/python/linting#_specific-linters
    //
    "python.linting.enabled": true,
    "python.linting.lintOnSave": true,
    "python.linting.pylintEnabled": false,
    "python.linting.flake8Enabled": true,
    "python.linting.flake8Path": "/Users/jm/opt/anaconda3/envs/sbi/bin/flake8",
    //
    // Files to exclude
    //
    "files.exclude": {
        "**/__pycache__": true,
        "**/.pytest_cache": true,
        "**/.ipynb_checkpoints": true,
        "**/*.egg-info": true,
    },
    //
    // Rewrap plugin
    // Needs manual installation (!)
    // https://marketplace.visualstudio.com/items?itemName=stkb.rewrap
    //
    "rewrap.autoWrap.enabled": true,
    "rewrap.wrappingColumn": 88,
    //
    // autoDocstring plugin
    // Needs manual installation (!)
    // https://marketplace.visualstudio.com/items?itemName=njpwerner.autodocstring
    //
    "autoDocstring.customTemplatePath": "/Users/jm/Library/Application Support/Code/User/autodocstring/Google_no_types.mustache",
}

Open issues are:

  • How to deal with paths for black/isort/flake8 which are likely installed in a conda or venv environment? Can VSCode find them without a fixed path?
  • How to deal with the paths for setup.cfg, possibly just making it relative works here?
  • How to make sure that plugins such as Rewrap and autodDocstring are installed?
  • autoDocstring template should be included as well Inconsistent indication of types in docstrings #75 (comment)
  • VScode settings #128 (comment)
  • Should go into a PR (feel free to grab the initial config above and build one from it!)
@jan-matthis jan-matthis added enhancement New feature or request good first issue Good for newcomers help wanted Extra attention is needed labels Apr 16, 2020
@janfb
Copy link
Contributor

janfb commented Apr 16, 2020

Thanks! what are your /Users/jm/sbi/setup.cfg settings?

@jan-matthis
Copy link
Contributor Author

jan-matthis commented Apr 16, 2020

Thanks! what are your /Users/jm/sbi/setup.cfg settings?

/Users/jm/sbi is my local path to the sbi repository (or rather a symlink to it, but that doesn't matter). So, same setup.cfg that is part of the repo.

We should get rid of absolute paths for our shared config.

@jan-matthis
Copy link
Contributor Author

Thanks! and where and how do you refer to autodocstring.template in vscode?

The relevant part of the config for autoDocstring is:

"autoDocstring.customTemplatePath": "/Users/jm/Library/Application Support/Code/User/autodocstring/Google_no_types.mustache"

We should make the path relative and also include the template from #75 (comment)

@janfb
Copy link
Contributor

janfb commented Apr 16, 2020

Thanks! what are your /Users/jm/sbi/setup.cfg settings?

/Users/jm/sbi is my local path to the sbi repository (or rather a symlink to it, but that doesn't matter). So, same setup.cfg that is part of the repo.

We should get rid of absolute paths for our shared config.

Ah, I see. But then this should be added for flake8 as well, no? e.g., something like

"python.linting.args": [
        "--settings-path=/Users/jm/sbi/setup.cfg"
    ],

@jan-matthis
Copy link
Contributor Author

jan-matthis commented Apr 16, 2020

Thanks! what are your /Users/jm/sbi/setup.cfg settings?

/Users/jm/sbi is my local path to the sbi repository (or rather a symlink to it, but that doesn't matter). So, same setup.cfg that is part of the repo.
We should get rid of absolute paths for our shared config.

Ah, I see. But then this should be added for flake8 as well, no? e.g., something like

"python.linting.args": [
        "--settings-path=/Users/jm/sbi/setup.cfg"
    ],

I think VSCode+flake8, in contrast to VScode+isort, might be able to find and use the setup.cfg automatically. But yes, I think it wouldn't hurt to include it just to be sure.

@jan-matthis
Copy link
Contributor Author

jan-matthis commented Apr 16, 2020

I updated the first post to incorporate a couple of changes and add a list of open issues.

@janfb
Copy link
Contributor

janfb commented Apr 16, 2020

Great, thanks!
I use those settings now, but the docstrings are not formatted on save yet, only when I do it by hand using alt-q.
Does that work for you?

@jan-matthis
Copy link
Contributor Author

It does not rewrap existing lines without pressing alt-q. But if I type a new line and exceed the bound, a line break is inserted

@alvorithm
Copy link
Contributor

  • My path to black is just 'black' - works well if installed in the conda environment, we can control the version
$ black --version
black, version 19.10b0
  • same for flake8path, just flake8
  • I have no path fixed to isort, VSCode says 'default using inner version'.
  • Arguments to isort are
    "python.sortImports.args": [
        "-rc",
        "-w 88",
        "-m 3",
    ],
  • Arguments to black:
    "python.formatting.blackArgs": [
        "--line-length=88"
    ],
  • however I have commented out the following (remember there were headaches, we had to turn it off.. @janfb do you remember why exactlly?
    // "[python]": {
    //     "editor.codeActionsOnSave": {
    //         "source.organizeImports": true
    //     },
    // },
  • I also have
    "editor.wordWrapColumn": 88,
    "editor.codeActionsOnSave": null,  // different to JM
  • For rewrap, same settings as JM above
  • Other perhaps relevant settings
    "editor.renderWhitespace": "boundary",
    "editor.wordWrap": "wordWrapColumn",
    "editor.fontLigatures": true,
  • Linting:
    "python.linting.pylintArgs": [
        "--generated-members=numpy.* ,torch.*",
        "--disable=invalid-name"
  • In my sbi repo's .vscode/settins.json I have
{
    "python.pythonPath": "~/Local/miniconda3/envs/sbi/bin/python",
    "python.testing.pytestArgs": [
        "tests"
    ],
    "python.testing.unittestEnabled": false,
    "python.testing.nosetestsEnabled": false,
    "python.testing.pytestEnabled": true
}
  • If we version that file, I don't know how to read the pythonPath from somewhere else.

@jan-matthis
Copy link
Contributor Author

jan-matthis commented Apr 16, 2020

I found flake8 to work much better than pylint (e.g., you do not need to specify generated members), guess there is no need for both?

Regarding isort I figured it is important to specify the path to setup.cfg. In the isort section of it known third_party packages are listed explicitly, which gets rid of unexpected behaviour -- in particular this open issue on isort: PyCQA/isort#1147. Maybe that is also the reason why you ended up turning off "source.organizeImports": true?

@alvorithm
Copy link
Contributor

OK, I am sold - let's try the way you suggest.

@alvorithm
Copy link
Contributor

Why don't you push your file as .vscode/settings.json except for the paths (see my comment) and we take it from there? (PR with line-level comments?).

The only thing I wouldn't know how to do portably is setting the pythonPath, unless each of us makes some sort of local alias...

@alvorithm
Copy link
Contributor

The only thing I wouldn't know how to do portably is setting the pythonPath, unless each of us makes some sort of local alias...

Maybe we could use this and keep it unversioned: https://code.visualstudio.com/docs/python/environments#_environment-variable-definitions-file

@alvorithm
Copy link
Contributor

Concretely the trick would be to define PYTHON_SBI there, then use in .vscode/settings.json the following

"python.pythonPath": "${env:PYTHON_SBI}",

@alvorithm
Copy link
Contributor

Why don't you push your file as .vscode/settings.json except for the paths (see my comment) and we take it from there? (PR with line-level comments?).

@jan-matthis I guess I am taking this over.

@alvorithm
Copy link
Contributor

(as an orthogonal, but related element of pleasurable coding, I can recommend the JetBrains mono font which is readably beautiful and open source. @janfb @jan-matthis @michaeldeistler).

@jan-matthis
Copy link
Contributor Author

@jan-matthis I guess I am taking this over.

Yes, would be great if you did a PR and we comment back on forth on it then

Concretely the trick would be to define PYTHON_SBI there, then use in .vscode/settings.json the following `"python.pythonPath": "${env:PYTHON_SBI}"

That's definitely an option. What would be the pro/con for not defining python.pythonPath in the shared config at all?

@jan-matthis
Copy link
Contributor Author

(Unrelated to the config discussion: This is a great feature of VSCode that I'm starting to appreciate more and more: https://code.visualstudio.com/docs/editor/tasks)

@alvorithm
Copy link
Contributor

What would be the pro/con for not defining python.pythonPath in the shared config at all?

Each of us has a different conda environment with a different conda location. Unless we apply the trick of nesting the conda env in the repo, then .gitignori'ng it.

@alvorithm
Copy link
Contributor

(Unrelated to the config discussion: This is a great feature of VSCode that I'm starting to appreciate more and more: https://code.visualstudio.com/docs/editor/tasks)

Thanks for sharing. Do you have a concrete automation in mind for sbi?

@jan-matthis
Copy link
Contributor Author

Thanks for sharing. Do you have a concrete automation in mind for sbi?

I'm using tasks to run files in a tmux session -- it's pretty hacky at this point though and not specific to sbi

@alvorithm alvorithm linked a pull request May 1, 2020 that will close this issue
@alvorithm alvorithm added this to the Post-release cleanup milestone May 4, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request good first issue Good for newcomers help wanted Extra attention is needed
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants