airspeed-velocity / asv

Airspeed Velocity: A simple Python benchmarking tool with web-based reporting
https://asv.readthedocs.io/
BSD 3-Clause "New" or "Revised" License
845 stars 177 forks source link

MAINT: Vendor `mamba.api` #1372

Closed HaoZeke closed 5 months ago

HaoZeke commented 5 months ago

Closes #1353. Also addresses comments in #1370. There's an upstream clarification issue in https://github.com/mamba-org/mamba/issues/3159. However, I think this makes sense to go in as-is. Will cut a release soon with the fix.

HaoZeke commented 5 months ago

Might also be of interest to @dstansby and / or @hmaarrfk 😃

hmaarrfk commented 5 months ago

if this helps resolve the challenges with context not being initialized correctly that is great!

I will say that now that conda-libmamba-solver has been deployed, there is little advantage of using "mamba", and you may want to just "avoid churn".

Thanks for keeping this project working all these years!

HaoZeke commented 5 months ago

if this helps resolve the challenges with context not being initialized correctly that is great!

That bug-fix is next, this is to get the mamba solver up and running again. I was wondering if you had a reproducible example I could play with where the .condarc needs to be read and isn't.

I will say that now that conda-libmamba-solver has been deployed, there is little advantage of using "mamba", and you may want to just "avoid churn".

I understand conda uses libmamba as an alternate backend, but for a while (couple of releases) I will probably keep the plugin in (also in my tests conda is still a bit slower).

Thanks for keeping this project working all these years!

Glad its still in use! (though I'm a relatively new maintainer, 7-8 months tops I guess).

HaoZeke commented 5 months ago

Draft for a bit, the plugin is still rather fragile (the most compatible way to use it seems to be to keep mamba.api as is and enforce that the user resolves dependencies via micromamba install libmambapy conda conda-build mamba.

Otherwise (without mamba.api):

·· Failure creating environment for mamba-py3.9-numpy1.21
Traceback (most recent call last):
  File "/home/rgoswami/micromamba/envs/asv_exp/bin/asv", line 8, in <module>
    sys.exit(main())
  File "/home/asvT/asv/asv/main.py", line 29, in main
    result = args.func(args)
  File "/home/asvT/asv/asv/commands/__init__.py", line 49, in run_from_args
    return cls.run_from_conf_args(conf, args)
  File "/home/asvT/asv/asv/commands/run.py", line 166, in run_from_conf_args
    return cls.run(
  File "/home/asvT/asv/asv/commands/run.py", line 278, in run
    Setup.perform_setup(environments, parallel=parallel)
  File "/home/asvT/asv/asv/commands/setup.py", line 82, in perform_setup
    list(map(_create, environments))
  File "/home/asvT/asv/asv/commands/setup.py", line 14, in _create
    env.create()
  File "/home/asvT/asv/asv/environment.py", line 740, in create
    self._setup()
  File "/home/asvT/asv/asv/plugins/mamba.py", line 130, in _setup
    solver = MambaSolver(
  File "/home/asvT/asv/asv/plugins/_mamba_helpers.py", line 156, in __init__
    self.index = load_channels(
  File "/home/asvT/asv/asv/plugins/_mamba_helpers.py", line 141, in load_channels
    repo = subdir.create_repo(pool)
libmambapy.bindings.MambaNativeException: Cache not loaded
HaoZeke commented 5 months ago

Should be good to go now. Works with only libmambapy and conda installed.

HaoZeke commented 5 months ago

@mattip does this look alright? I'm thinking of this, #1375 and #1373 along with a minor documentation update (taking in the asv-memray plugin) for a release this weekend if everything goes well.

mattip commented 5 months ago

The upstream issue says, in justifying removing the API functions:

no more logic will be done on the Python side.

and

do not hesitate to report here all the APIs you need and you cannot find in the libmambapy folder

It seems some of the functionality here should be reported upstream as missing, so that we can get rid of the helper class eventually.

mattip commented 5 months ago

Is there a test that was failing before this PR and now passes? I don't see any test changes.

HaoZeke commented 5 months ago

The upstream issue says, in justifying removing the API functions:

no more logic will be done on the Python side.

and

do not hesitate to report here all the APIs you need and you cannot find in the libmambapy folder

It seems some of the functionality here should be reported upstream as missing, so that we can get rid of the helper class eventually.

My reading of that was just that the API functionality needed (also in boa) was meant to be handled at this level, the MambaSolver class is part of the documentation and is noted as an "example of using libmambapy" and subsequently needed tweaking (liberally lifted from boa) but yes it would be great to remove this eventually.

HaoZeke commented 5 months ago

Is there a test that was failing before this PR and now passes? I don't see any test changes.

I hadn't written a CI test for mamba before, so I've added that now for this PR.

mattip commented 5 months ago

Thanks @HaoZeke. mamba test is passing