search

letmaik / lensfunpy

📷 Lens distortion correction for Python, a wrapper for lensfun
https://pypi.python.org/pypi/lensfunpy
MIT License
140 stars 18 forks source link

Prioritise user-defined databases over packaged version #35

Closed tvwerkhoven closed 2 years ago

tvwerkhoven commented 2 years ago

In: https://github.com/letmaik/lensfunpy/blob/a16273a5488c9124cf5f035092cae031df213933/lensfunpy/_lensfun.pyx#L245

Propose to prioritise user-defined databases over packaged version. The former is most likely better.

From:

paths += glob.glob(xml_glob)

To:

paths = glob.glob(xml_glob) + paths
letmaik commented 2 years ago

Can you confirm whether lf_db_load_file ignores entries if they already exist in the database? Meaning, it doesn't override them? I'm assuming the former, hence the issue, but would be good to have some reference for this.

letmaik commented 2 years ago

Note that you can also disable loading of packaged lenses. Would that solve your issue?

tvwerkhoven commented 2 years ago
  1. No lf_db_load_file does not ignore, but overwrites existing entries.
  2. load_bundled=False seems to work, but it's a bit inconvenient if you want to update just 1 lens.

When adding a database file manually it's first in the list, e.g. after loading the db, the paths list looks like:

['/opt/local/share/lensfun/version_1/slr-tamron.xml',
 '/Users/tim/Library/Python/3.8/lib/python/site-packages/lensfunpy/db_files/compact-canon.xml',
 '/Users/tim/Library/Python/3.8/lib/python/site-packages/lensfunpy/db_files/slr-samsung.xml',
 '/Users/tim/Library/Python/3.8/lib/python/site-packages/lensfunpy/db_files/slr-olympus.xml',
 '/Users/tim/Library/Python/3.8/lib/python/site-packages/lensfunpy/db_files/slr-nikon.xml',
 '/Users/tim/Library/Python/3.8/lib/python/site-packages/lensfunpy/db_files/compact-kodak.xml',
 '/Users/tim/Library/Python/3.8/lib/python/site-packages/lensfunpy/db_files/compact-sigma.xml',
 '/Users/tim/Library/Python/3.8/lib/python/site-packages/lensfunpy/db_files/compact-ricoh.xml',
...

however, it seems the last file in the list is prioritised, as the extra lens parameters I added to my custom file disappeared.

load_bundled=False seems to work, but it's a bit inconvenient if you have 1 lens to update because you need to load other files as well. E.g. I want to use vignetting info of Tamron SP 24-70mm f/2.8 Di VC USD (which is optically the same as the Pentax, hence my original question), so if I update slr-tamron.xml with vignetting info. I'd prefer to load just that and rely on the rest of the db for other lenses/camera, else I would need to load all the other files I need as well.

I think changing the load order would fix this and provide a more user-friendly flow.

letmaik commented 2 years ago

Looking at this in a bigger context, this is the current loading order, each overriding previous matching entries:

  1. paths argument (default is empty)
  2. bundled files, if load_bundled=True (default is true)
  3. xml argument (default empty)
  4. common folders, if load_common=True (default is true)

It seems like a more sensible order would be:

  1. bundled files, if load_bundled=True (default is true)
  2. common folders, if load_common=True (default is true)
  3. paths argument (default is empty)
  4. xml argument (default empty)
tvwerkhoven commented 2 years ago

Agree, that makes sense.

A nice addition would be if paths can be globbed as well, e.g. paths=['/opt/local/share/lensfun/version_1/*tamron.xml'], similarly how load_bundled uses glob.glob(xml_glob), but perhaps that should go in another issue ;)

letmaik commented 2 years ago

I pushed a new version of lensfunpy with the loading order change, see https://github.com/letmaik/lensfunpy/releases/tag/v1.11.0. It'll be up on PyPI once https://github.com/letmaik/lensfunpy/actions/runs/2905440721 ran through.