feat: add base flag to info command #3779

SandrineP · 2025-01-28T16:59:25Z

Adds one of the missing sub-command #3535

jjerphan · 2025-01-31T07:53:12Z

libmamba/src/api/info.cpp

+                std::cout << ctx.prefix_params.root_prefix.string() << std::endl;
            }
+            else


We should be able to early return in this case, also LOG_INFO is preferred generally I think.

Suggested change

std::cout << ctx.prefix_params.root_prefix.string() << std::endl;

}

else

LOG_INFO << ctx.prefix_params.root_prefix.string() << std::endl;

return;

}

and remove the parenthesis of the blocks and unindent the code to use the previous identation.

I don't understand which parenthesis you are talking about.
If I just make this change (replacing std::cout with LOG_INFO and adding return;), the output is not printed in the terminal

My bad: I meant curly brackets, and let's use std::cout.

So I leave it as is?

Actually part of my previous comment can be addressed: can you early return on the first case so that the block from line 99 to line 193 can keep the same level of indentation?

jjerphan · 2025-01-31T08:02:29Z

micromamba/src/info.cpp

+    auto& base = config.insert(
+        Configurable("base", false).group("cli").description("Display base environment path.")
+    );
+    subcom->add_flag("--base", base.get_cli_config<bool>(), base.description());


Can mamba info --base be tested?

I have no idea how to test it. Open to suggestions!

What do you think of testing that the usual information is not present and that an existing path is output?

jjerphan · 2025-01-31T08:20:21Z

micromamba/src/info.cpp

+using namespace mamba;
+


What is this used for?

It's for Configurable, which is a mamba::Configurable.

OK, in this case I propose https://github.com/mamba-org/mamba/pull/3779/files#r1936931327.

Indeed, using namespace directive should be avoided as much as possible.

It is done in /mamba/micromamba/src/list.cpp , which is why I originally went for it in /mamba/micromamba/src/info.cpp (but it used only once here, and more in list.cpp).

OK, we can remove (broad) using namespace directives in another PR.

jjerphan · 2025-01-31T09:25:33Z

micromamba/src/info.cpp

@@ -22,6 +25,11 @@ set_info_command(CLI::App* subcom, mamba::Configuration& config)
    init_info_parser(subcom, config);
    static bool print_licenses;

+    auto& base = config.insert(
+        Configurable("base", false).group("cli").description("Display base environment path.")


How about using this if this is possible?

Suggested change

Configurable("base", false).group("cli").description("Display base environment path.")

mamba::Configurable("base", false).group("cli").description("Display base environment path.")

Co-authored-by: Klaim <Klaim@users.noreply.github.com>

Signed-off-by: Julien Jerphanion <git@jjerphan.xyz> Co-authored-by: Johan Mabille <johan.mabille@gmail.com> Co-authored-by: Hind Montassif <hind.montassif@gmail.com>

…mamba-org#3792) Signed-off-by: Julien Jerphanion <git@jjerphan.xyz>

Signed-off-by: Julien Jerphanion <git@jjerphan.xyz>

…rg#3788) Signed-off-by: Julien Jerphanion <git@jjerphan.xyz>

jjerphan · 2025-02-12T16:50:18Z

libmamba/src/api/info.cpp

    namespace detail
    {
+        struct list_options
+        {
+            bool base;


Suggested change

bool base;

bool base = false;

jjerphan · 2025-02-12T16:52:37Z

libmamba/src/api/info.cpp

+                std::cout << ctx.prefix_params.root_prefix.string() << std::endl;
            }
+            else


Actually part of my previous comment can be addressed: can you early return on the first case so that the block from line 99 to line 193 can keep the same level of indentation?

jjerphan · 2025-02-12T16:53:11Z

micromamba/tests/test_info.py

+        "platform",
+    ]
+    if base_flag == "--base":
+        print(infos)


Suggested change

print(infos)

jjerphan · 2025-02-12T16:54:01Z

micromamba/tests/test_info.py

+        assert all(f"{i} :" not in infos for i in items)
+        assert os.path.exists(infos.strip())
+    else:
+        assert all(i in infos for i in items)


Suggested change

assert all(i in infos for i in items)

assert all(f"{i} :" in infos for i in items)

jjerphan · 2025-02-12T16:56:15Z

micromamba/tests/test_info.py

+        assert all(f"{i} :" not in infos for i in items)
+        assert os.path.exists(infos.strip())


Suggested change

assert all(f"{i} :" not in infos for i in items)

assert os.path.exists(infos.strip())

assert all(f"{i} :" not in infos for i in items)

base_environment_path = infos.strip()

assert os.path.exists(base_environment_path)

jjerphan · 2025-02-12T17:02:17Z

micromamba/tests/test_info.py

+def test_base_subcommand(tmp_home, tmp_root_prefix, prefix_selection, base_flag):
+    os.environ["CONDA_PREFIX"] = str(tmp_root_prefix)


pytest exposes a monkeypatch fixture which can be used to monkeypatch environment variable in the scope of a test. This is a bit safer and the intend is clearer.

Suggested change

def test_base_subcommand(tmp_home, tmp_root_prefix, prefix_selection, base_flag):

os.environ["CONDA_PREFIX"] = str(tmp_root_prefix)

def test_base_subcommand(tmp_home, tmp_root_prefix, prefix_selection, base_flag, monkey_patch):

monkeypatch.setenv("CONDA_PREFIX", str(tmp_root_prefix))

Not that this suggestion might not respect linters' formatting.

add base subcommand

670491d

SandrineP force-pushed the info_base branch from 44bc606 to 670491d Compare January 30, 2025 11:57

SandrineP added the release::enhancements For enhancements PRs or implementing features label Jan 30, 2025

SandrineP marked this pull request as ready for review January 30, 2025 16:19

jjerphan reviewed Jan 31, 2025

View reviewed changes

SandrineP mentioned this pull request Feb 4, 2025

Missing command options in micromamba #3535

Open

2 tasks

jjerphan and others added 14 commits February 6, 2025 15:00

maint: Factor handling of GetModuleFileNameW (mamba-org#3785)

3abf53c

Co-authored-by: Klaim <Klaim@users.noreply.github.com>

release libmamba 2.0.6.rc2, micromamba 2.0.6.rc2, libmambapy 2.0.6.rc2

2532535

feat: add canonical flag to list command (mamba-org#3777)

fd4116d

ci: Rerun pytest tests on main in case of failures (mamba-org#3769)

109b47f

Signed-off-by: Julien Jerphanion <git@jjerphan.xyz> Co-authored-by: Johan Mabille <johan.mabille@gmail.com> Co-authored-by: Hind Montassif <hind.montassif@gmail.com>

fix: Use libmamba's installation instead of mamba's as a fallback (…

d2a444c

…mamba-org#3792) Signed-off-by: Julien Jerphanion <git@jjerphan.xyz>

ci: Fix typo in Windows workflows (mamba-org#3793)

dac50d8

Signed-off-by: Julien Jerphanion <git@jjerphan.xyz>

maint: Warn about future removal of etc/profile.d/mamba.sh (mamba-o…

f201df2

…rg#3788) Signed-off-by: Julien Jerphanion <git@jjerphan.xyz>

release libmamba 2.0.6.rc3, micromamba 2.0.6.rc3, libmambapy 2.0.6.rc3

4ef7677

release libmamba 2.0.6, micromamba 2.0.6, libmambapy 2.0.6

1747cd8

feat: add export flag to list command (mamba-org#3780)

bc0b7db

minor fix

f7d55c8

add base test

c3f7a42

Merge branch 'main' into info_base

ed90b5d

edit test

d473b05

SandrineP force-pushed the info_base branch from b4a2cab to d473b05 Compare February 6, 2025 15:50

jjerphan reviewed Feb 12, 2025

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: add base flag to info command #3779

feat: add base flag to info command #3779

SandrineP commented Jan 28, 2025

jjerphan Jan 31, 2025

SandrineP Jan 31, 2025

jjerphan Jan 31, 2025

SandrineP Jan 31, 2025

jjerphan Jan 31, 2025

jjerphan Feb 12, 2025

jjerphan Jan 31, 2025

SandrineP Jan 31, 2025

jjerphan Jan 31, 2025

jjerphan Jan 31, 2025

SandrineP Jan 31, 2025

jjerphan Jan 31, 2025

JohanMabille Feb 5, 2025

SandrineP Feb 5, 2025

jjerphan Feb 5, 2025

jjerphan Jan 31, 2025

jjerphan Feb 12, 2025

jjerphan Feb 12, 2025

jjerphan Feb 12, 2025

jjerphan Feb 12, 2025

jjerphan Feb 12, 2025

jjerphan Feb 12, 2025

	Configurable("base", false).group("cli").description("Display base environment path.")
	mamba::Configurable("base", false).group("cli").description("Display base environment path.")

	assert all(i in infos for i in items)
	assert all(f"{i} :" in infos for i in items)

		assert all(f"{i} :" not in infos for i in items)
		assert os.path.exists(infos.strip())

		def test_base_subcommand(tmp_home, tmp_root_prefix, prefix_selection, base_flag):
		os.environ["CONDA_PREFIX"] = str(tmp_root_prefix)

feat: add base flag to info command #3779

Are you sure you want to change the base?

feat: add base flag to info command #3779

Conversation

SandrineP commented Jan 28, 2025

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment