log images and array-like data by saarah815 · Pull Request #70 · neuroinformatics-unit/fancylog

saarah815 · 2025-08-08T17:49:00Z

Description

What is this PR

Bug fix
Addition of a new feature
Other

What does this PR do?

Allows user to use fancylog to log images, as well as array-like data such as rotation matrices.

References

#65
#68

How has this PR been tested?

Tests have been added to test_general.py.

Is this a breaking change?

No.

Does this PR require an update to the documentation?

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

IgorTatarnikov

I think tifffile and numpy need to be added to the pyproject.toml as dependencies (perhaps as optional dependencies, if we want to keep this package light weight).

The code looks great, would it be possible to allow log_image and log_data_object to be called without having to provide the logging_dir every time? Each running log is stored in the logging module, you can access the dictionary of current loggers using logging.getLoggerClass().manager.loggerDict, each logger then has a list of handlers where the first one is the file that's being written (typically), you can fetch it with curr_logger.handlers[0].baseFilename.

Otherwise, it might be worth it to adjust fancylog to remember it's state somehow. Passing the logging directory on each log_image call feels like it could lead to some unexpected behaviour.

saarah815 · 2025-08-18T11:43:07Z

Thoughts on defaulting to the current logger, but leaving logging_dir as an optional argument in case the user wants the images elsewhere?

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

I think tifffile and numpy need to be added to the pyproject.toml as dependencies (perhaps as optional dependencies, if we want to keep this package light weight).

+1 for optional dependencies

saarah815 · 2025-08-18T18:06:19Z

I grouped tifffile and numpy as image extras in pyproject.toml:

image = [
    "numpy",
    "tifffile",
]

So that it can be installed with pip install fancylog[image] for those who need it.

…like_data

codecov · 2025-08-19T16:12:44Z

Codecov Report

❌ Patch coverage is 89.58333% with 5 lines in your changes missing coverage. Please review.
✅ Project coverage is 79.50%. Comparing base (a7b04bd) to head (6b369e5).

Files with missing lines	Patch %	Lines
fancylog/fancylog.py	89.36%	5 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main      #70      +/-   ##
==========================================
+ Coverage   77.15%   79.50%   +2.35%     
==========================================
  Files           3        3              
  Lines         197      244      +47     
==========================================
+ Hits          152      194      +42     
- Misses         45       50       +5

☔ 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.

IgorTatarnikov

This is great! Just a few more comments and then I think it's good to go.

fancylog/fancylog.py

pyproject.toml

tests/tests/test_general.py

saarah815 · 2025-08-27T01:39:29Z

Thank you @IgorTatarnikov! I've just pushed with all changes implemented.

…able (unlike /dev)

IgorTatarnikov

This is almost there @saarah815. I've left comments throughout. Let me know if you don't have the bandwidth to work on this and I can finish it off.

IgorTatarnikov · 2026-02-16T14:08:40Z

fancylog/fancylog.py

+import numpy as np
+import tifffile


These shouldn't be imported at the top of the file. they should be local to the function that uses them. If the user doesn't have the optional libraries installed this will lead to a crash.

IgorTatarnikov · 2026-02-16T14:10:20Z

fancylog/fancylog.py

+                f"Falling back to '{logging_dir}'"
+            )
+
+    output_dir = Path(logging_dir)


For consistency this should also expanduser and resolve, just like in log_image.

IgorTatarnikov · 2026-02-16T15:12:10Z

tests/tests/test_general.py

    log_file = next(tmp_path.glob("*.log"))

-    try:
+    with open(log_file) as file:


I'm not sure why this test has changed.

This should be it's own PR if possible.

IgorTatarnikov · 2026-02-16T15:13:28Z

tests/tests/test_general.py

-def test_multiprocessing_warning_on_windows(tmp_path):
-    """A warning is raised and multiprocessing logging
-    is disabled on Windows.
+def test_log_image_creates_tiff_and_metadata(tmp_path, caplog):
+    """Test that log_image writes a TIFF
+    and optional metadata file.
    """
-    with (
-        patch("platform.system", return_value="Windows"),
-        pytest.warns(
-            UserWarning, match="Multiprocessing logging is not supported"
-        ),
-    ):
-        fancylog.start_logging(
-            tmp_path,
-            fancylog,
-            multiprocessing_aware=True,


The test_multiprocessing_warning_on_windws was deleted and should be restored (I think this is a result of merge conflicts)

IgorTatarnikov · 2026-02-16T15:46:23Z

fancylog/fancylog.py

+        filepath = data_dir / f"{name}.npy"
+        np.save(filepath, data)
+    else:
+        raise ValueError("Unsupported data type for logging")


This error could be more descriptive, the user should be informed the type they provided to ease debugging.

IgorTatarnikov · 2026-02-16T16:08:08Z

tests/tests/test_general.py

+    assert (
+        "[fancylog] No default logging directory found. Falling back to"
+        in message
+        for message in caplog.text
+    )


This doesn't actually test anything as of right now. The statement inside the brackets returns a generator which is always True (I think). This can be simplified to the suggestion based on https://docs.pytest.org/en/stable/how-to/logging.html

Suggested change

assert (

"[fancylog] No default logging directory found. Falling back to"

in message

for message in caplog.text

)

assert "[fancylog] No default logging directory found. Falling back to" in caplog.text

IgorTatarnikov · 2026-02-16T16:10:11Z

tests/tests/test_general.py

+    monkeypatch.setattr(fancylog, "get_default_logging_dir", lambda: None)
+    monkeypatch.setattr(type(Path()), "cwd", classmethod(lambda cls: tmp_path))
+    monkeypatch.setattr(fancylog.fancylog, "_RUN_TIMESTAMP", "test_timestamp")


This test is causing some issues because the monkeypatch is not actually successful here.

You're looking to patch the specific functions being called. It feels a bit odd at first, but you're specifying that the Path class imported in fancylog.fancylog should have it's cwd method replaced with something that return tmp_path.

Suggested change

monkeypatch.setattr(fancylog, "get_default_logging_dir", lambda: None)

monkeypatch.setattr(type(Path()), "cwd", classmethod(lambda cls: tmp_path))

monkeypatch.setattr(fancylog.fancylog, "_RUN_TIMESTAMP", "test_timestamp")

monkeypatch.setattr(fancylog.fancylog, "get_default_logging_dir", lambda: None)

monkeypatch.setattr(fancylog.fancylog.Path, "cwd", classmethod(lambda cls: tmp_path))

monkeypatch.setattr(fancylog.fancylog, "_RUN_TIMESTAMP", "test_timestamp")

IgorTatarnikov · 2026-02-16T16:11:30Z

pyproject.toml

 user_support = "https://github.com/neuroinformatics-unit/fancylog/issues"

 [project.optional-dependencies]
 dev = [


The dev install should be "batteries included". The easiest way to do this would be to add fancylog[git, image, array, multiprocessing] to the dev optional dependencies. This way there's one source of truth, and installing the package for testing is as simple as possible!

log images and array-like data

1982302

adamltyson marked this pull request as ready for review August 11, 2025 07:33

adamltyson marked this pull request as draft August 11, 2025 07:33

saarah815 added 2 commits August 11, 2025 18:27

save as tiff

9eff334

tests for log_image and log_data_object

da57ea7

saarah815 marked this pull request as ready for review August 12, 2025 14:44

IgorTatarnikov reviewed Aug 18, 2025

View reviewed changes

change logging_dir to optional, add timestamp to folder name

54b9e59

saarah815 and others added 4 commits August 19, 2025 16:14

install image extra in test env

5404c98

Merge remote-tracking branch 'origin/main' into log_images_and_array_…

a91fb98

…like_data

resolve merge conflict

2b4855b

Merge branch 'main' into log_images_and_array_like_data

8de9ab4

ensure tests cover all functionality

6b369e5

IgorTatarnikov requested changes Aug 26, 2025

View reviewed changes

update fallback when no logging_dir found

6bdb934

saarah815 and others added 3 commits February 5, 2026 14:31

Merge branch 'main' into log_images_and_array_like_data

024f179

test packages against pip list instead of importlib.metadata

b10d98e

avoid permission error and ensure fallback logging dir is always writ…

22d2705

…able (unlike /dev)

IgorTatarnikov requested changes Feb 16, 2026

View reviewed changes

		import numpy as np
		import tifffile

Conversation

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

Uh oh!

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!

IgorTatarnikov left a comment

Choose a reason for hiding this comment

Uh oh!

saarah815 commented Aug 18, 2025

Uh oh!

adamltyson commented Aug 18, 2025

Uh oh!

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

Uh oh!

Uh oh!

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

Uh oh!

Codecov Report

Uh oh!

IgorTatarnikov left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

saarah815 commented Aug 27, 2025

Uh oh!

IgorTatarnikov 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!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

saarah815 commented Aug 8, 2025 •

edited

Loading

saarah815 commented Aug 18, 2025 •

edited

Loading

codecov bot commented Aug 19, 2025 •

edited

Loading