Skip to content

Conversation

@drammock
Copy link
Member

@drammock drammock commented Jul 7, 2020

closes #7945

@drammock drammock changed the title update install instructions DOC: update install instructions Jul 7, 2020
@cbrnr
Copy link
Contributor

cbrnr commented Jul 7, 2020

Looks very nice! Other than my tiny nitpick and one failing Azure job 👍 for merge.

Copy link
Member

@hoechenberger hoechenberger left a comment

Choose a reason for hiding this comment

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

Left two small comments, otherwise looks good to me

@@ -1,4 +1,4 @@
name: base
name: mne
Copy link
Member

Choose a reason for hiding this comment

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

You're going to need to update the CIs to reflect this change, we assume base in our azure-pipelines.yml at least

https://github.com/mne-tools/mne-python/blob/master/azure-pipelines.yml#L150

Copy link
Member Author

Choose a reason for hiding this comment

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

The particular line you're linking to is in the notebook/server CI job, so it actually should stay the way it is. But there are other CI changes needed; working on them now.

@larsoner larsoner added the backport-candidate on-merge: backport to maint/1.11 label Jul 7, 2020
@larsoner
Copy link
Member

larsoner commented Jul 7, 2020

Marking as backport-candidate. We don't need to make a release for this, but do need to at least push a commit to maint/0.20 so that mne.tools/stable gets updated with these instructions, since we always tell people to use the master version of environment.yml

I did not look too deeply at the changes, happy to look once @cbrnr and @hoechenberger are happy (or if you need another opinion)

@drammock
Copy link
Member Author

drammock commented Jul 8, 2020

Here's the rendered doc: https://20983-1301584-gh.circle-artifacts.com/0/dev/install/mne_python.html

Marking as WIP because I had to remove spyder-kernels. If the CIs come back green, I'll try re-adding it via pip instead of conda (in local testing, installing it with pip didn't cause it to pull in a stupidly low version of qt)

@drammock drammock changed the title DOC: update install instructions WIP, DOC: update install instructions Jul 8, 2020
Comment on lines 304 to 310
- `Visual Studio Code`_ (often shortened to "vscode") is a development-focused
text editor that supports many programming languages in addition to Python,
and has a rich ecosystem of packages to extend its capabilities. Installing
`Microsoft's Python Extension
includes an integrated terminal console, and has a rich ecosystem of packages
to extend its capabilities. Installing `Microsoft's Python Extension
<https://marketplace.visualstudio.com/items?itemName=ms-python.python>`__ is
enough to get most Python users up and running. Visual Studio Code is free
and open-source.
Copy link
Member

@hoechenberger hoechenberger Jul 8, 2020

Choose a reason for hiding this comment

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

I had the idea to also add the "VS Code" abbreviation, WDYT?

- `Visual Studio Code`_ (often shortened to "VS Code" or "vscode") is a
  development-focused text editor that supports many programming languages in
  addition to Python, includes an integrated terminal console, and has a rich
  ecosystem of packages to extend its capabilities. Installing
  `Microsoft's Python Extension
  <https://marketplace.visualstudio.com/items?itemName=ms-python.python>`__ is
  enough to get most Python users up and running. Visual Studio Code is free
  and open-source.

Also, one thing I'd love us to mention is the Pylance extension, even though it's still a preview version. It builds on the Python extension and for me, this thing was really a game changer: for the first time since I started using VS Code in Python, I feel that it's actually working properly, and is fun to use. It would make using Python so much better, even for newcomers:

Link to the extension:
https://marketplace.visualstudio.com/items?itemName=ms-python.vscode-pylance

Pylance announcement:
https://devblogs.microsoft.com/python/announcing-pylance-fast-feature-rich-language-support-for-python-in-visual-studio-code/#comments

Copy link
Contributor

Choose a reason for hiding this comment

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

It's just sad that VS Code still can't do proper Python indentation, e.g.:

x = [1, 2, 3,
4, 5, 6]

Of course there's a dedicated extension, but this basic feature should really work out of the box. Other than that, VS Code is great, and so is PyCharm.

Copy link
Member

Choose a reason for hiding this comment

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

It's just sad that VS Code still can't do proper Python indentation, e.g.:

+1000

Other than that, VS Code is great, and so is PyCharm.

I still prefer PyCharm Professional to VS Code :) But VS Code is catching up, quick.

Copy link
Member

@hoechenberger hoechenberger left a comment

Choose a reason for hiding this comment

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

Good to go from my end besides the comment I left.

@drammock drammock changed the title WIP, DOC: update install instructions MRG, DOC: update install instructions Jul 8, 2020
@drammock
Copy link
Member Author

drammock commented Jul 8, 2020

Looks like using pip for spyder-kernels cleared things up. Ready for merge.

@hoechenberger I didn't end up adding the pylance info, since I personally don't feel comfortable recommending it because (1) I haven't actually tested it, and (2) it's closed source and they don't plan to change that. LMK if including pylance is a blocker for you; if so I can find a way to mention it without endorsing explicitly.

@larsoner
Copy link
Member

larsoner commented Jul 8, 2020

So far pylance is a bit buggy in my experience, +1 for holding off on recommending it

@hoechenberger
Copy link
Member

Ok for me to merge as-is!

@cbrnr cbrnr merged commit 2965cc3 into mne-tools:master Jul 8, 2020
@cbrnr
Copy link
Contributor

cbrnr commented Jul 8, 2020

Thanks @drammock!

@drammock drammock deleted the update-install-instructions branch July 8, 2020 16:51
@larsoner larsoner added backported and removed backport-candidate on-merge: backport to maint/1.11 labels Nov 5, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Trouble installing Spyder into MNE conda environment

4 participants