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

Fix mamba environment option #1370

Closed
wants to merge 3 commits into from

Conversation

dstansby
Copy link
Contributor

@dstansby dstansby commented Jan 17, 2024

I'm attempting to fix #1353. Currently this involves:

  • adding some extra docs
  • adding an import guard to check that libmambapy is installed
  • making the mamba plugin discoverable by asv (by importing it in plugins.__init__.py)

However, I'm not convinced the mamba option has ever worked... The mamba Python API does not contain mamba.api.MambaSolver(), which was added as an import in #1238. @HaoZeke do you know any more about this import and what package needs to be installed to access it?

@HaoZeke HaoZeke self-requested a review January 17, 2024 16:05
@HaoZeke
Copy link
Member

HaoZeke commented Jan 17, 2024

@HaoZeke do you know any more about this import and what package needs to be installed to access it?

I will take a closer look after next week (am traveling). It was certainly available at the time of that PR, though libmambapy has changed quite a bit over the past year or so.

@HaoZeke
Copy link
Member

HaoZeke commented Jan 27, 2024

So there used to be a mamba.api which shipped a default implementation of MambaSolver as noted in an older fork. It was removed in December of 2023. However it has been removed so I'll setup a different default implementation for us (boa moves far too quickly and is too heavy to be a dependency).

Comment on lines +3 to +5
# Import to make sure plugins are discoverable
import asv.plugins.conda
import asv.plugins.mamba
Copy link
Member

Choose a reason for hiding this comment

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

This shouldn't be required? (plugins are auto-discovered, failures mean they are dropped from valid options)

Copy link
Member

Choose a reason for hiding this comment

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

Rather than explicitly raise from here the current approach is to not let mamba be a valid option for an environment if the import fails.

The reason behind this is to allow for external plugins (it is also why we don't want to have an explicit list of allowed values).

Comment on lines +83 to +84
but needs a newer Python version (3.8 or greater) and the ``libmambapy``
package must be installed to provide access to the mamba Python API.
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
but needs a newer Python version (3.8 or greater) and the ``libmambapy``
package must be installed to provide access to the mamba Python API.
but needs a newer Python version (3.8 or greater) and the ``libmambapy``
package must be installed (typically through ``conda``) to provide access to the mamba Python API.

Clarification since libmambapy is not present on PyPI.

@HaoZeke
Copy link
Member

HaoZeke commented Feb 1, 2024

Closing in favor of #1372 and #1373.

@HaoZeke HaoZeke closed this Feb 1, 2024
@dstansby dstansby deleted the fix-mamba branch February 1, 2024 20:13
@dstansby
Copy link
Contributor Author

dstansby commented Feb 1, 2024

Fab, thanks for picking this up!

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.

BUG: "environment_type": "mamba" fails
2 participants