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.

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

Jump to bottom

MRG: Simplify manual installation instructions a little by dropping explicit mention of (lib)mamba #12362

Merged
merged 6 commits into from
Apr 12, 2024
Merged

MRG: Simplify manual installation instructions a little by dropping explicit mention of (lib)mamba #12362

merged 6 commits into from
Apr 12, 2024

Conversation

hoechenberger
Copy link
Member

@hoechenberger hoechenberger commented Jan 15, 2024 •
edited
Loading

Also use the channel::package syntax in two places instead of --channel=... package. I think it's more compact.

FYI "strict" channel priority will become the default in a not-so-far-away future version of conda, so we will then be able to drop the respective parameters as well, making the command lines even shorter.

@hoechenberger
Simplify manual install docs a little by dropping explicit mention of…
ad7f63d 
… (lib)mamba

Also use the `channel::package` syntax in two places instead of `--channel=... package`. I think it's more compact.
@hoechenberger hoechenberger changed the title Simplify manual install docs a little by dropping explicit mention of (lib)mamba Simplify manual install instructions a little by dropping explicit mention of (lib)mamba Jan 15, 2024
@hoechenberger hoechenberger changed the title Simplify manual install instructions a little by dropping explicit mention of (lib)mamba Simplify manual installation instructions a little by dropping explicit mention of (lib)mamba Jan 15, 2024
drammock
drammock reviewed Jan 15, 2024
View reviewed changes
doc/install/manual_install.rst Outdated
Comment on lines 17 to 18
Ensure you are using a recent version of ``conda``, otherwise the dependency resolution
may take extremely long or even fail!
Copy link
Member

Choose a reason for hiding this comment

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

can we be specific here about which conda version is the recommended minimum version? (presumably this is the first one where libmamba is the default solver and where the channel::package syntax is supported).

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 channel::package syntax was introduced in conda 4.4.0, released in Dec 2017:
https://docs.conda.io/projects/conda/en/4.6.0/release-notes.html#id71

Since it's so old, I will not mention this

The switch of the default solver to libmamba OTOH happened only a few months ago, so I will mention the specific version number

Copy link
Member Author

Choose a reason for hiding this comment

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

I reverted the channel::package syntax change, since I'm not 100% sure it always functions the way we intend

Copy link
Member Author

Choose a reason for hiding this comment

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

@drammock I've addressed the issue of using a recent-enough conda version in af16862

@hoechenberger hoechenberger added this to the 1.7 milestone Apr 9, 2024
@hoechenberger
Copy link
Member Author

this should go into 1.7

@larsoner
Copy link
Member

larsoner commented Apr 9, 2024

Good to go from your end @hoechenberger ?

@hoechenberger
Copy link
Member Author

hoechenberger commented Apr 9, 2024 •
edited
Loading

no, this is not ready to be merged!

@hoechenberger
Copy link
Member Author

hoechenberger commented Apr 9, 2024 •
edited
Loading

i'm on mobile now, would you be so kind as to change this to a draft PR please?

@larsoner larsoner marked this pull request as draft April 9, 2024 14:21
@hoechenberger hoechenberger marked this pull request as ready for review April 12, 2024 14:16
@hoechenberger hoechenberger changed the title Simplify manual installation instructions a little by dropping explicit mention of (lib)mamba MRG: Simplify manual installation instructions a little by dropping explicit mention of (lib)mamba Apr 12, 2024
@hoechenberger
Copy link
Member Author

@larsoner This is ready to go from my side. If the installation page comes back rendered nicely, it should be good to merge.

drammock
drammock approved these changes Apr 12, 2024
View reviewed changes
Copy link
Member

@drammock drammock left a comment

Choose a reason for hiding this comment

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

LGTM: https://output.circle-artifacts.com/output/job/bc7eb00d-36f1-438b-9ecf-79fcb9856e6c/artifacts/0/html/install/manual_install.html#manual-install

@drammock drammock enabled auto-merge (squash) April 12, 2024 14:50
@larsoner larsoner disabled auto-merge April 12, 2024 16:33
@larsoner
Copy link
Member

Failure is due to scikit-learn/scikit-learn#28826 so I'll manually merge. I might open a PR to work around it but let's see what the sklearn people say today -- it could be a one-day issue.

@larsoner larsoner merged commit a6f0331 into mne-tools:main Apr 12, 2024
27 of 30 checks passed
@hoechenberger hoechenberger deleted the install-docs branch April 12, 2024 16:36
@hoechenberger
Copy link
Member Author

great, thanks 🙏

larsoner added a commit to larsoner/mne-python that referenced this pull request Apr 15, 2024
@larsoner
Merge remote-tracking branch 'upstream/main' into rank
455a3dc 
* upstream/main: (50 commits)
  ENH: Improve OPM auditory dataset and example (mne-tools#12539)
  MAINT: Bump to latest pydata-sphinx-theme (mne-tools#12228)
  MRG: Simplify manual installation instructions a little by dropping explicit mention of (lib)mamba (mne-tools#12362)
  fix PSD weights handling when bad annotations present (mne-tools#12538)
  Fix phase loading (mne-tools#12537)
  align FFT windows to good data spans in psd_array_welch (mne-tools#12536)
  explicitly disallow multitaper in presence of bad annotations (mne-tools#12535)
  MAINT: Clean up PyVista contexts (mne-tools#12533)
  MAINT: Complete API change of ordered (mne-tools#12534)
  MAINT: Reinstall statsmodels and improve logging (mne-tools#12532)
  MAINT: Remove scipy.signal.morlet2 (mne-tools#12531)
  Update README badge links (mne-tools#12529)
  BUG: Fix bug with reading his_id from snirf (mne-tools#12526)
  [pre-commit.ci] pre-commit autoupdate (mne-tools#12524)
  Fix file format check in _check_eeglab_fname function (mne-tools#12523)
  MAINT: Reenable picard in pre testing (mne-tools#12525)
  MAINT: Bump to large resource class (mne-tools#12522)
  MAINT: Restore 2 jobs on Windows (mne-tools#12520)
  Add exclude_after_unique option to mne.io.read_raw_edf/read_raw_edf to search for exclude channels after making channel names unique (mne-tools#12518)
  Improve consistency of sensor types in code and documentation (mne-tools#12509)
  ...
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@drammock drammock drammock approved these changes

@larsoner larsoner larsoner approved these changes

Assignees
No one assigned
Labels
DOC no-changelog-entry-needed
Projects
None yet
Milestone
1.7
Development

Successfully merging this pull request may close these issues.
3 participants
@hoechenberger @larsoner @drammock