Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Provide visibility for an active fork #31

Open
wants to merge 283 commits into
base: master
Choose a base branch
from
Open

Conversation

ickc
Copy link

@ickc ickc commented Dec 24, 2019

While this is unlikely to be merged, this PR provides some visibility to an active fork that has merged many other PR from here.

@smarnach
Copy link
Owner

I'm not really sure what to do about this library. I currently don't have time to maintain it, but I'm happy to spend some time for a proper handover if we can find a new owner for the code. (I spun this off a StackOverflow answe and never meant to really maintain it, but it unexpectedly became somewhat popular.)

@sylikc Would you like to become the "official" owner of the code? Would anyone else like?

Things we should do to hand this over:

  • Add a link to the maintained fork in the README of this repo.
  • Try to get ownership of the PyPI packages someone created for this.
  • Get external links updated, e.g. the one from my StackOverflow answer.
  • Update the documentation link in the README to no longer point to the documentation hosted here.

@ickc
Copy link
Author

ickc commented Dec 27, 2019

There’s also a way to transfer in GitHub to make his “official” and others forks of his. I forgot exactly how but it can be done.

@ickc
Copy link
Author

ickc commented Jan 5, 2020

I made 3 PR to @sylikc ~11 days ago. But there's no activity there. Looking at @sylikc/pyexiftool I found that all the new commits happened in July 2019 so I'm not sure if he's still actively maintaining that.

I recently wrote a tool (so far for personal use only) that depends on pyexiftool. It is very likely that I'll continue to use my tool and hence continue to improve pyexiftool since there's seem no other better way (my tool needs to know the metadata from raw so exiftool is uniquely capable on that.) And unless I move to another language or write my own code to basically does what pyexiftool does then I should be able to continue to maintain that.

And I currently maintained a few packages on PyPI so it is likely I can continue to keep the packages up-to-date in PyPI including CI on newer versions of Python. (I might add a conda package for it too since for conda environment it is more convenient.)

I think if @sylikc wants to, since he has longer history on using that tool and already done a dozen merges on the PRs here, he should be the first choice. But if we don't hear from him soon we may interpret that he's not actively maintaining that either. Then I'm willing to pick it up.

…ckc-support-w

as per manual merge instructions
adding ickc feature for no_output from GitHub PR #3
@sylikc
Copy link

sylikc commented Feb 2, 2020

I am happy to maintain this. When I have free time, I'm actively developing my own scripts to accelerate my photography workflow. I have an internal gitlab so haven't been as active checking GitHub thinking no one would use it :)

Thanks for the enhancements @ickc and thanks for making the code available and doing all the initial legwork @smarnach

I'm aware one of the forks in some way or form is in PyPI which of course is many versions behind what PR I've merged from this project.

@sylikc
Copy link

sylikc commented Feb 2, 2020

I don't have the experience with PyPI, but I'd be willing to learn from you @ickc how to get the package up, and maintain it.

@ickc
Copy link
Author

ickc commented Feb 6, 2020

Sounds like a plan. If @smarnach agree then may be @smarnach could transfer the ownership of GitHub to @sylikc first?

About PyPI, currently on PyPI PyExifTool is owned by intense.feel/Martin Carnogursky in https://pypi.org/user/intense.feel/. Do you know his GitHub username? If we can't contact him then we could just have it in an alternative name (not uncommon when they have multiple forks.)

@RootLUG
Copy link

RootLUG commented Feb 6, 2020

That's me on PyPI, I was going to upload the forked & modified version but forgot to change the package name before uploading so I took the package name by accident. Please let me know the PyPI name of the new maintainer and with a confirmation from @smarnach I will transfer the ownership of the package on PyPI.

@sylikc
Copy link

sylikc commented Feb 6, 2020

I created an account on PyPI to prepare for a transfer. same username.

@smarnach
Copy link
Owner

smarnach commented Feb 6, 2020

@sylikc @RootLUG Thanks a lot, sounds all good to me. It will take me a few days until I can find time to update links etc, but I will eventually get there.

@sylikc
Copy link

sylikc commented Feb 7, 2020

cool @smarnach how do I set up the githubio page for the documentation? is there an automated process to generate the docs?

@smarnach
Copy link
Owner

Sorry about the delay! This is still on my list, but I'm really busy at the moment. :-/

@sylikc
Copy link

sylikc commented Apr 9, 2020

Is there any update on this @smarnach ?

@ickc I've reviewed other PyPI modules and there's no standard for renaming things to be py-something so I'm going to roll back that specific commit. I'm going to be working with the library again since we're all stuck at home... I'll try not to break any interfaces if I add features ;)

sylikc and others added 6 commits April 9, 2020 04:29
This reverts commit de22a96.

This reverts the rename of "exiftool" to "pyexiftool" at the suggestion of the community.  The exiftool name will remain for all of eternity.
…47/pyexiftool into csparker247-fix-nonetype-exception

resolve bug in using the wrong common_args in the constructor.  in Pull request #5 by @csparker247
…ical chunks in a directory.

This should help with code review clearer moving forward.  I'm not a big fan of huge source files...

The static helper functions (which rarely/should not change) will be isolated from the ExifTool-specific class

(tests pass)
…... so when it crashes here, there's no guessing why
… check some type of errorlevel from exiftool
@JSpenced
Copy link

JSpenced commented Jul 18, 2020

@smarnach Any news on this? Be nice to have the forked version in PyPI. Thanks for all the effort so far.

@melyux
Copy link

melyux commented Oct 11, 2020

@smarnach please

@RootLUG
Copy link

RootLUG commented Oct 16, 2020

I see that there are multiple repos being forked with some of them more updated than others in terms of pull requests.
So let's maybe form a plan or otherwise we can discuss for another year before moving anywhere forward with this issue.
my proposal:
let's first agree on what repo should the code continue forward and would be the authority to publish the package on pypi. Is it this one or some of the forks mentioned in the issue? Maybe the forks are also very different and we should create a new repo where we rebase and merge all the changes from those forks + pending pull requests etc; basically, prepare the code for release.
I can then very easily add a CI pipeline that would auto-publish the package on pypi based on whatever is in master branch on that repo that we would agree on. At that point, we can formulate some contribution guidelines and maintenance of the code (incoming pull requests) so we can get regular updates which would be automatically released via that CI pipeline.

That's of course just my opinion so if you guys have better plan, objections or suggestions please share 😄

@RootLUG
Copy link

RootLUG commented Oct 16, 2020

regarding the documentation, to me it looks like normal github-pages with static HTML. You can see it in https://github.com/smarnach/pyexiftool/tree/gh-pages so if you just generate sphinx docs into gh-pages branch it would publish that on github.io as static page.

@ickc
Copy link
Author

ickc commented Oct 16, 2020

When I evaluated it 10 months ago, the sylikc repo is most mature. He basically merged all the PR in this repo into his, and reflected in his CHANGELOG.md. There was a questionable renaming in his 0.4 but after communicating with him he rolled that back in 0.4.2.

I think his fork is pretty solid, unless if you have something better and can point us to look at it.

I think doing the ownership transfer and set up the necessary CI and PyPI is not the problem—they are very easy to do.

I think @smarnach already agreed to this. The hard work is how and when it happens.

  1. transfer GitHub ownership of the repo
  2. PyPI ownership transfer

Frankly I don't quite care about (2). Multiple forks of a package on PyPI is not new (even pyyaml has this.) (1) is much more important to inform people which repo is now the "authoritative" one to use. (1) can be easily done. Regarding the PyPI version, to me it suffices to say "package A is deprecated and please upgrade yours by installing its drop-in replacement by pip uninstall ... && pip install ...."

I think @sylikc's intention is to have a drop-in replacement of this as can be shown by his effort to rolled the back change such that no changes on the user-side has to happen.

@sylikc
Copy link

sylikc commented Oct 20, 2020

I'm happy to continue to improve PyExifTool . @RootLUG what other forks have features that I could/should pull into my fork?

With @ickc 's direction, I certainly want to maintain compatibility as much as possible and keep it a drop-in replacement for the library. As I edit/add features/calls, I am certainly mindful of not changing the existing behavior.

So, I can change the description on my project to say "up to date" or something. I guess I could set up another PyPI to have it installable through pip? Is that the preferred direction?

@ickc
Copy link
Author

ickc commented Oct 20, 2020

Just to clarify, I think unnecessary breaking change should be avoided, but in the (unlikely) event that a breaking change is needed, a clear communication with a major version increment would suffices.

My suggestion given the amount of time has passed already, would be to start a new PyPI repo maintained by @sylikc (with whatever different name deemed appropriate.) When @smarnach has the time then the only thing he need to do is to change the ownership of this repo to @sylikc. In the meanwhile, I hope this PR has given that fork a visibility enough for people to migrate smoothly.

sylikc and others added 30 commits April 27, 2022 23:04
…perly... now all tests updated to test this behavior
* update _is_iterable() to have an ignore_str_bytes parameter
* execute() uses list comprehension instead of the if-else.  Renamed the params list to make it clearer
* get_tags() and set_tags() files/params are allowed to be arbitrary objects.  let execute() handle the casting

Add tests to get full coverage.  Fix the params test before
Keep a list of bytes instead of repeatedly appending (copying) to the bytes
object.
…uotes (when possible, like not with f-strings)... then fixed a bit of previous code with the same styling difference
…st to (lazy) fix the 3.6 arch x64 problem.

I think there's other better fixes, but the thread is super long actions/setup-python#544
This PR addresses the append() operations and uses lists and reverse() to speed up getting exiftool output

Merge PR #61 by https://github.com/prutschman
No functional changes, only a speed optimization of existing code.
…om json.loads() - documentation not complete yet for method, and naming is not final

ExifToolHelper - updated some documentation.
…ethod the way it's intended, to workaround exiftool JSON float handling glitch
* add documentation for the set_json_loads() metdho
* Fix some flake8 styling and unused variables linting
* update all copyright years to 2023
* update CHANGELOG.md for 0.5.6 release
* prepare for release
…hes. Add a try/except so it works properly in CPython 3.12.0
FAQ
- fix a typo in set_json_loads() FAQ
- add a section in FAQ to use shlex to help debug argument splitting.  Lots of issues are opened just on how to split arguments.
- refer to the FAQ from the quickstart/examples page

small tweak to the changelog
helper.py TUPLE_STR_BYTES, while functionally the same, should be str, bytes

constants.py, put DEFAULT_EXECUTABLE into a one-liner so that it's properly documented by AutoAPI
removed accidentally added file from a few commits ago (no need to rewrite history as it's just a random text file with links to resources)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.