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 micromamba env export to get channel name instead of full url #2260

Merged
merged 2 commits into from
Feb 22, 2023

Conversation

Hind-M
Copy link
Member

@Hind-M Hind-M commented Feb 2, 2023

Fix #2235
In case of pkgs/main, defaults is used to be consistent with conda.

@jonashaag
Copy link
Collaborator

In case of pkgs/main, defaults is used to be consistent with conda.

Hmm... is this a special case in the Conda source code? If so, could you post a link to the piece of code that does it? I'm curious to see how exactly it is implemented.

@Hind-M
Copy link
Member Author

Hind-M commented Feb 6, 2023

Hmm... is this a special case in the Conda source code? If so, could you post a link to the piece of code that does it? I'm curious to see how exactly it is implemented.

Actually, I just run conda env export in a test environment which printed:

channels:
  - conda-forge
  - defaults

And looking at the Conda source code, I found this:
https://github.com/conda/conda/blob/fc92ea3353cce71df24fac31ce333ec9f8cf7ba7/tests/models/test_index_record.py#L30-L31
But didn't find the explicit implementation of it...

@jonashaag
Copy link
Collaborator

I'd be interested if Conda always prints the name of the channel that has been used to install (like --from-history), or the name of the multichannel.

Comment on lines 124 to 127
if (chan_name == "pkgs/main")
{
channels.insert("defaults");
}
Copy link
Member

Choose a reason for hiding this comment

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

It looks like using either pkgs/main or defaults works so I would be conservative and say we do not need to change? I'm no expert here. What do you think?

In the refactorign reflection, this should go in Channel, eventually through a new method name like cannonical_name so that we don't manage edge cases everywhere.

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes we could leave pkgs/main then (less prone to bugs and avoid edge cases). And yes, I agree that we should use something like canonical_name like Conda does IIRC.

@@ -120,7 +120,7 @@ def test_env_export(self):
create("", "-n", env_name, "-f", spec_file)
ret = yaml.safe_load(run_env("export", "-n", env_name))
assert ret["name"] == env_name
assert set(ret["channels"]) == {"https://conda.anaconda.org/conda-forge"}
assert set(ret["channels"]) == {"conda-forge"}
Copy link
Member

Choose a reason for hiding this comment

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

If we do add that the defaults naming, we should also add a test for it, so that we do not loose it through a refactoring.

Copy link
Member Author

Choose a reason for hiding this comment

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

Indeed.

@jonashaag
Copy link
Collaborator

I'd be interested if Conda always prints the name of the channel that has been used to install (like --from-history), or the name of the multichannel.

Did anyone check this?

@Hind-M
Copy link
Member Author

Hind-M commented Feb 9, 2023

I'd be interested if Conda always prints the name of the channel that has been used to install (like --from-history), or the name of the multichannel.

Did anyone check this?

Nope. I've looked a bit into their code base but it wasn't obvious to spot and have the full picture. I would probably spend more time on it later if no one else checks it or has already some prior background to narrow the search.

@jonashaag
Copy link
Collaborator

OK! Should we wait until we have a definite answer? I guess solving this issue isn't urgent, and if we change behavior twice that might be confusing to users.

@Hind-M
Copy link
Member Author

Hind-M commented Feb 9, 2023

Yes, that seems reasonable!

@jonashaag
Copy link
Collaborator

I had a look:

$ cat ~/.condarc
channels:
- defaults
- conda-forge

custom_multichannels:
  my_multi:
  - conda-forge

$ mamba create -n t1 conda-forge::zlib
$ mamba create -n t2 defaults::zlib  # => does not work
$ mamba create -n t2 pkgs/main::zlib
$ mamba create -n t3 my_multi::zlib

$ conda env export -n t1
name: t1
channels:
  - my_multi
  - defaults
  - conda-forge
...

$ conda env export -n t2
name: t2
channels:
  - defaults
  - conda-forge
  # => NOTE: No my_multi
...

$ conda env export -n t3
name: t3
channels:
  - my_multi
  - defaults
  - conda-forge
...

$ micromamba env export -p /opt/conda/envs/t1
name:
channels:
- https://conda.anaconda.org/conda-forge
- https://repo.anaconda.com/pkgs/main
# => NOTE: No my_multi, but mamba env export has it
...

$ micromamba env export -p /opt/conda/envs/t2
name:
channels:
- https://repo.anaconda.com/pkgs/main
# => NOTE: No conda-forge and my_multi, but mamba env export have it
...

$ micromamba env export -p /opt/conda/envs/t3
name:
channels:
- https://conda.anaconda.org/conda-forge
- https://repo.anaconda.com/pkgs/main
# => NOTE: No my_multi, but mamba env export has it
...

Now with --from-history:

$ conda env export -n t1 --from-history
name: t1
channels:
  - defaults
  - conda-forge
dependencies:
  - conda-forge::zlib
prefix: /opt/conda/envs/t1

$ conda env export -n t2 --from-history
name: t2
channels:
  - defaults
  - conda-forge
dependencies:
  - pkgs/main::zlib
prefix: /opt/conda/envs/t2

$ conda env export -n t3 --from-history
name: t3
channels:
  - defaults
  - conda-forge
dependencies:
  - my_multi::zlib
prefix: /opt/conda/envs/t3

$ micromamba env export -p /opt/conda/envs/t1 --from-history
name:
channels:
- https://conda.anaconda.org/conda-forge
# => NOTE: No defaults, but mamba env export has it
dependencies:
- conda-forge::zlib

$ micromamba env export -p /opt/conda/envs/t2 --from-history
name:
channels:
- https://repo.anaconda.com/pkgs/main
# => NOTE: No conda-forge, but mamba env export has it
dependencies:
- pkgs/main::zlib

$ micromamba env export -p /opt/conda/envs/t3 --from-history
name:
channels:
- https://conda.anaconda.org/conda-forge
# => NOTE: No defaults, but mamba env export has it
dependencies:
- my_multi::zlib

IIUC we should always print the multichannel name in channels:, and not special-case defaults.

@jonashaag
Copy link
Collaborator

Interestingly, the order of channels is different with Micromamba. Is that a bug? cc @wolfv

@Hind-M
Copy link
Member Author

Hind-M commented Feb 20, 2023

Thanks!
So, it looks like using pkgs/main over defaults is the way to go indeed mainly because of this:

$ mamba create -n t2 defaults::zlib # => does not work

But I still find the general logic not that clear:

  • When using --from-history, the printed channels should be the ones used explicitly when getting packages, so the behavior of micromamba seems more logical to me (should we just follow conda's behavior for consistency?).
    This is how other channels are inserted when exporting without the from-history flag, and (when ignore_channels flag is set to False (default). Note that this flag doesn't exist in micromamba apparently):
    https://github.com/conda/conda/blob/main/conda_env/env.py#L128-L133

  • Otherwise, when exporting without any flags, I'm wondering if conda considers the condarc file and only prints custom_multichannels if they match the ones used when getting packages? (prints my_multi only because it's set to conda-forge and when the package is requested from there).

But anyway, I think that any other changes should be done in other PRs since they would be considered as additional features (handle custom_multichannels and the other missing ones).

Copy link
Collaborator

@jonashaag jonashaag left a comment

Choose a reason for hiding this comment

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

Agreed!

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.

micromamba env export exports full channel URLs
3 participants