Log env packages #62

svalbuenaa · 2025-08-02T10:54:44Z

Description

What is this PR

Bug fix
Addition of a new feature
Other

Why is this PR needed?
Include logging python version and environment packages as requested in issue #14

What does this PR do?
Two new methods (write_python_version and write_environment_packages) have been added to the LoggingHeader class.

write_python_version uses sys.version to logthe python version of the interpreter
write_environment_packages attempts to log the conda environment name and conda environment packages (using a subprocess for the latter). Note that, in case fancylog is run within a conda environment, only the packages available in this environment are reported (as conda does not offer a way to use base environment packages from within an environment). In case there is no conda environment associated to the execution of fancylog (i.e. calling os.environ["CONDA_PREFIX"] results in a KeyError), the method falls back to log the local and globally available packages. To do so, it uses two subprocesses (one calling pip list --local for the locally available packages and another one calling pip list for the globally available packages. write_environment_packages logs all the locally available packages and any globally available package that is not also present in the local environment.

References

Issue #14

How has this PR been tested?

Six new tests are added.
test_python_version_header_absent asserts that there is no PYTHON VERSION section header when fancylog.start_logging is called with write_python=False.
test_python_version_header_present asserts that there is a PYTHON VERSION section header when fancylog.start_logging is called with write_python=True.
test_correct_python_version_logged asserts that the python version logged is correct. The write_python_version method logs the python version with sys.version. The test compares the logged version with the output of calling platform.python_version()
test_environment_header_absent asserts that there is no ENVIRONMENT section header when fancylog.start_logging is called with write_env_packages=False.
test_environment_header_present asserts that there is no ENVIRONMENT section header when fancylog.start_logging is called with write_env_packages=True.
test_correct_pkg_version_logged If fancylog is run within a conda environment, the test asserts that the version logged for each package reported by conda list is logged. Note that this way of accessing the versions of packages in the conda environment is the same that the write_environment_packages uses to log the packages. If there is no conda environment the logged version of local environment packages, obtained through a subprocess call to pip list --local) is compared with the version obtained through importlib.metadata.distributions.

Is this a breaking change?

There is no breaking change. There is, however, a slow down in the logging process due to the calls to conda list or pip list, which take some time to be completed. This is mainly apparent in the time that it takes for unit tests to be completed.

Does this PR require an update to the documentation?

Methods and tests have been documented. No changes to README.md have been made, though.

Checklist:

The code has been tested locally
Tests have been added to cover all new functionality
The documentation has been updated to reflect any changes
The code has been formatted with pre-commit

adamltyson

Hey @svalbuenaa, this looks really nice, thanks! I've left a few comments about reducing duplication, and making sure all the code is tested in all environments.

I haven't had chance to look over every line yet, so I may have a few more comments.

adamltyson · 2025-08-04T12:50:12Z

fancylog/fancylog.py

+    write_python
+        Log the Python version. Default: True
+    write_env_packages
+        Log the packages in the conda/pip environment. Default: True


AFAIK there's no such thing as a "pip" env.

Suggested change

Log the packages in the conda/pip environment. Default: True

Log the packages in the environment. Default: True

You are right, sorry about this. I will remove it in the next implementation I submit

You can directly commit a GitHub review suggestion if you like, there's a button at the bottom of my comment.

adamltyson · 2025-08-04T12:52:37Z

fancylog/fancylog.py

+            for pkg in conda_pkgs:
+                self.file.write(f"{pkg['name']:20} {pkg['version']:15}\n")


Is it possible to just have one loop to reduce duplication of the code below?

There is already just one loop there, right? Should I try to remove it altogether?

Or perhaps you meant these other lines with a conditional inside a for loop?:

for pkg in global_env_pkgs: if pkg["name"] not in local_env_pkgs_names: self.file.write(f"{pkg['name']:20} {pkg['version']:15}\n")

yeah, i just meant there is some duplication. There's a similar (or identical) loop later on in the same file later on.

got it. I am finishing implementing this with a helper method to avoid redundancies. Besides, I am currently working on logging the local and the global packages with only one call to
subprocess.run(["pip", "list"])
so the new implementation is also faster

This is also implemented in the updated version

adamltyson · 2025-08-04T13:20:02Z

tests/tests/test_general.py

+    """Python version section does not exist if logger
+    is created with the parameter write_python_version as False.
+    """
+    ver_header = f"{lateral_separator}  PYTHON VERSION  {lateral_separator}\n"


Nitpicking, but this could be a pytest fixture to reduce duplication

I was not familiar with fixtures, thank you for the suggestion. I will try to combine the two tests this way

They can take some getting used to, but they're very helpful to create "things" to be reused.

By using
@pytest.mark.parametrize
decorators I can now combine the two pairs of header tests together

The updated version uses parametrization to combine the tests for the presence/absence of the headers

adamltyson · 2025-08-04T13:29:46Z

tests/tests/test_general.py

+        conda_exe = os.environ["CONDA_EXE"]
+        conda_list = subprocess.run(
+            [conda_exe, "list", "--json"], capture_output=True, text=True


can this be tested in a non-conda env somehow? It won't currently be tested in our GitHub actions. Perhaps the return values could be mocked?

I have tested this in both conda and venv. In my hands works well in both cases (i.e. when it is run in a conda env it logs the conda env packages and when it is run out of a conda env, such as in a venv, the appropriate packages are also logged). I will look into mocking return values and be back with an implementation

Sorry I should have been clear, the tests run fine in all environments, but the issue is that if you're in a conda environment, the pip version won't be tested (and vice versa). This is particularly important with our automated tests that run in GitHub actions. It doesn't use conda, and so the conda version of the code could end up broken for a long time without us noticing.

Currently the the tests run some kind of system command, and then make sure the right section is added to the log. We don't really care if e.g. pip list returns the right thing, that's someone elses job to test! We only care about testing fancylog code here. You can override what will be returned by these system commands to "pretend" you're in a specific environment, and then test the fancylog code as needed. There are a few ways of doing this, such as setting environmental variables, but the general process would be:

Set up the environment to "appear" a certain way

Run fancylog function, expecting it to run according to how the environment "appears"

Check that everything runs properly

This may not be that clear, so let me know if I help any further.

Thank you! I am currently working on this, I hope I have a new version with all the changes tomorrow or on Thursday

I just commited changes, adding two new tests, one of which mocks the existence of a conda environment, and asserts the presence in the log of the appropriate texts, and another which mocks the lack of a conda environment.

duplication when logging pkgs Add tests mocking environment, remove duplication writing packages

codecov · 2025-08-07T09:24:12Z

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 77.15%. Comparing base (d359f5d) to head (fbce37d).
⚠️ Report is 1 commits behind head on main.

Additional details and impacted files

@@            Coverage Diff             @@
##             main      #62      +/-   ##
==========================================
+ Coverage   71.51%   77.15%   +5.63%     
==========================================
  Files           3        3              
  Lines         158      197      +39     
==========================================
+ Hits          113      152      +39     
  Misses         45       45

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

svalbuenaa · 2025-08-07T12:55:12Z

I just added a new test to mock the lack of any local environment, so the patch coverage should be 100% now. Besides, my fork was 1 commit behind fancylog's main, so I merged this commit as well.

adamltyson

Thanks @svalbuenaa, this is great! I'm going to make a new release of fancylog now so we can start using it in other projects!

svalbuenaa · 2025-08-11T15:04:13Z

Thank you so much, @adamltyson. Looking forward to new contributions!

svalbuenaa added 2 commits July 30, 2025 17:28

Log python version

7021e8a

Log environment info

47d2c7c

adamltyson linked an issue Aug 4, 2025 that may be closed by this pull request

Report Python version (and maybe whole environment) #14

Closed

adamltyson requested changes Aug 4, 2025

View reviewed changes

Parametrize header tests, add tests mocking environment, remove

efb12fe

duplication when logging pkgs Add tests mocking environment, remove duplication writing packages

adamltyson self-requested a review August 7, 2025 09:23

svalbuenaa added 2 commits August 7, 2025 13:15

Merge remote-tracking branch 'upstream/main' into log_env_packages

646f62f

Add test to mock lack of any local environment

fbce37d

svalbuenaa force-pushed the log_env_packages branch from d591bf9 to fbce37d Compare August 7, 2025 12:51

adamltyson approved these changes Aug 7, 2025

View reviewed changes

adamltyson merged commit 99b2a5d into neuroinformatics-unit:main Aug 7, 2025
12 checks passed

adamltyson mentioned this pull request Aug 12, 2025

Latest versions of fancylog cause a crash with brainreg #71

Closed

JoeZiminski mentioned this pull request Sep 25, 2025

Add additional guards to write_environment_packages. #75

Merged

	Log the packages in the conda/pip environment. Default: True
	Log the packages in the environment. Default: True

		for pkg in conda_pkgs:
		self.file.write(f"{pkg['name']:20} {pkg['version']:15}\n")

Log env packages #62

Log env packages #62

Uh oh!

Conversation

svalbuenaa commented Aug 2, 2025

Description

References

How has this PR been tested?

Is this a breaking change?

Does this PR require an update to the documentation?

Checklist:

Uh oh!

adamltyson left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

codecov bot commented Aug 7, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

svalbuenaa commented Aug 7, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

adamltyson left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

svalbuenaa commented Aug 11, 2025

Uh oh!

Uh oh!

codecov bot commented Aug 7, 2025 •

edited

Loading

svalbuenaa commented Aug 7, 2025 •

edited

Loading