RhetTbull/osxphotos

osxphotos sync to also consider --merge location and folder structure

oPromessa opened this issue · 10 comments

oPromessa commented

Is your feature request related to a problem? Please describe.

  • GPS Location:
    • When performing osxphotos sync --import db --set all or --merge all the GPS location is not carried over from the original DB into the sync'ed DB.
  • Folder Structure:
    • Additionally the Album name is set correctly but the Folder structure is ignored, which osxphotos import does not ignore.

Describe the solution you'd like

  • GPS Location:
    • osxphotos --set / --merge all to also consider location
    • optionally: osxphotos --set / --merge location
    • With some pointers from you I could eventually try to work out the code changes into a PR 😄
  • Folder Structure:
    • osxphotos --set / --merge all to also consider folder structure by default
    • or osxphotos --set / --merge folder option
    • or osxphotos --set / --merge album/all --folder option
    • I imagine the code changes for this option might be a bit too much for me at this time to make a PR 😄

Describe alternatives you've considered

  • GPS Location:
    • osxphotos import --exportdb does this wonderfully, with the exceptions of i) taking some time and ii) AppleScript returning errors on import (some time overcome with rerun and --resue, sometimes not)
  • Folder Structure:
    • hmm... Manual. Aside from osxphotos import none I could thing of.

Additional context

RhetTbull commented

Good suggestions.

For location, it obviously can't be merged so the behavior would need to be determined for what happens if user uses --merge location. Currently the code doesn't have separate lists of properties for --set or --merge. I would recommend the behavior be the same as it is for favorite: if --merge and property is set in the destination then no action is taken; if property is not set in the destination but is set in the source, then the value is copied to destination.

The location field would need to be added to SYNC_IMPORT_TYPES:

osxphotos/osxphotos/cli/sync.py

Lines 52 to 58 in 9a07c29

SYNC_IMPORT_TYPES = [
"keywords",
"albums",
"title",
"description",
"favorite",
]

Then the import_metadata_for_photo function would need to be updated to handle this, either in a separate block like albums are handled:

https://github.com/RhetTbull/osxphotos/blob/9a07c29e50dd45c178eba5d890314729fa048997/osxphotos/cli/sync.py#L372C1-L375C79

Or in _set_metadata_for_photo and _merge_metadata_for_photo functions.

The folders/albums feature will take a bit more work and I'll have to dig in a bit more when I get a chance.

oPromessa commented
oPromessa commented

FYI only....

While trying to run --tests-sync I bumped into the get_os_version code in tests/conftest.py expecting to get my current OS version Sonoma's 14.6.1. I got 10.16 instead!!. Explained here: pythons-platform-mac-ver-reports-incorrect-macos-version.

I'm pretty sure you @RhetTbull are aware of SYSTEM_VERSION_COMPAT mode variable:

$ SYSTEM_VERSION_COMPAT=0 python3.11 -c 'import platform; print(platform.mac_ver())'
('14.6.1', ('', '', ''), 'x86_64')
$ SYSTEM_VERSION_COMPAT=1 python3.11 -c 'import platform; print(platform.mac_ver())'
('10.16', ('', '', ''), 'x86_64')

For now I'll edit tests/conftest.py to also accept OS_VER[1] == 16 to allow me to run the --tests-sync option locally.

osxphotos/tests/conftest.py

Lines 70 to 72 in 9a07c29

OS_VER = get_os_version() if is_macos else [None, None]
if OS_VER[0] == "10" and OS_VER[1] == "15":
# Catalina

oPromessa commented
  • I'm moving along nicely.
  • Changed sync.py and sync_results.py
  • Found issue #1641.
  • Changed conftest.py
  • Added location tests that run successfully locally with python -m pytest -vv --test-sync tests/test_cli_sync.py
  • Tests python -m pytest -vv PASS locally (python 3.11)
  • 🛑 But the Actions tests fail when running automatically on or around (weird!) tests/test_catalina_10_15_7.py::test_asdict. But re-running them one-by-one they PASS (for Mac... and Ubuntu).

    osxphotos/tests/test_catalina_10_15_7.py

    Line 1506 in 9a07c29

    def test_asdict(photosdb: osxphotos.PhotosDB):
  • 🎉1
RhetTbull commented

Great! I'll take a look at the failing tests when I get a chance. I responded to #1641 -- I don't believe it's actually a bug but intended behavior.

oPromessa commented

--set / --merge location feature added via #1642

  • 👍1
RhetTbull commented

Thanks! Should be merged in 0.68.5. I will work on the folder path part as I have time. Currently working on some changes to date handling so OSXPhotos can access the original date of an image even if it's been changed by the user in Photos.

oPromessa commented

Thanks! Should be merged in 0.68.5. I will work on the folder path part as I have time. Currently working on some changes to date handling so OSXPhotos can access the original date of an image even if it's been changed by the user in Photos.

Cool. Now that you mention dates; I was also thinking if sync should also set dates coming from the original database ?

RhetTbull commented

Yes that's a good idea.

RhetTbull commented

I'll work on adding the date + folder structure soon. Finishing some re-factoring of the date/time code to allow retrieval of the original date even if user has changed it. (Will allow osxphotos timewarp to reset dates, for example). I need to finish that code first.

My plan at the moment is to have albums or all sync folder structure by default as this avoids conflicts if user has Folder1/Album1 in one library and Album1 as a top level album in another -- photos get synced to the "right" album. With the code I posted to #1650 I could also remove photos from albums...make make that a separate option.

  • 🎉1