V0.5.x py3 refactor #13
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
fix inconsistent naming of class variables based on python conventions. use of _ (underscores) to designate private variables, and attributes to get and set attributes rather than direct with variable moving some of the default set lines to the top to see the defined class variables available to us added some comment breaks so the code is at least more readable to me added a "constants" file so we can refer to constants outside of the exiftool.py file
move the setting of the executable to be a property set, which can be changed restore the behavior which you can read-only the "running" property, but prevent setting it added test of attribute "running" and "executable"
make platform a constant so that there won't be a chance of typos comparing it in code
fixed self._process deletion. To be consistent, it is always defined, and will be None if the process is not running
…2344225 , remove the Python 2.x code with one liner (requires Python 3.3)
…o match the way subprocess library does it. (standardize the naming) fix all tests to pass remove the test that tests the find_executable() as it has been replaced with the standard library shutil.which()
…s back empty. add warning when terminating when Exiftool isn't running and now because of that, check before calling terminate()
This ensures that the return of {ready} is absolutely correct... in case a tag reads out "{ready}" or something, this is just a pre-emptive improvement
…t errors better by having an STDERR stream This has not yet been tested on LINUX. I will write tests later which tests this functionality in full on linux
…communicate() freezes and never finishes execution in this case on win32 platform
… timeout instead of wait_timeout
…ol dies or is killed, this is now detected by the property, rather than just relying on self._running
…ead() by telling exiftool to print out a synchronization token on stderr at the end of processing. This is similar to the {ready} sequence. This ensures there is ALWAYS data on stderr, and in turn, Windows will no longer block on the stderr read
moved out the code for the fd read to a static function so we can use the same one for stdout and stderr
… ExifToolHelper class currently contains the previous functionality in a verbatim way. No functionality was changed.
…list(str) and ALWAYS return a list straight from execute_json. This fixes a bug that if a wildcard is passed as the input to filename, the old way would just return the first file ([0]) which is incorrect.
…n but it can), filename is an int, and the the fsencode blows up
… get_metadata. changed the way to check for an iterable, so that you can pass arbitrary iterables to the function
changes the way tests are run against both the base class and the helper class
…ntially do the same thing. get_metadata is a special case alias of get_tags
merge the conflicts since the code has changed quite a bit and merge in the changes into helper.py instead of the main ExifTool class # Conflicts: # exiftool/exiftool.py
changed the config_file setting code, to use the property.setter to do error checking added a check of exiftool running prior to clearing the .run() method
|
🤗 I got the tests to pass! Do you think this is ready for prime time? @jangop @csparker247 |
|
Awesome, great work. 👍 We should make sure the additional 3 tests that are currently expected to fail no longer fail. And see that the documentation makes sense. |
|
for sure the documentation doesn't make sense... lol and I see residual Python 2 code I'll be removing later I've been updating "some" of the documentation in the sense of files like changelog and readme and compatibility, but for sure even some of the comments i had to update which aren't really up to the features that were changed or added |
…f this came to be. Turned it into UTF-8
… feature works right now) updated some of the comments and sample code to match what currently would be
… pretty at the moment, but it should have the core changes which v0.5.0 provides
trying to format better, added in the special thanks, and reordered some sections/split into subsections
while testing, realized that an invalid params would cause weird problems, so added error checking to params in Helper test_helper.py - added test and fixed failing tests
|
👍 🎉 Seems great. You've poured lots of time into this! Nice job! |
the output of tests changed and ignore some IDE stuff
* Add in exceptions classes and made all tests pass. Added a -w test. ExifToolHelper override run() to remove warning just like terminate()
* fixed bug which created random temp directories int he wrong place
|
Alright the time is upon us... for me to break everyone's existing code on upgrade! 😁😂😎 I'm working on documentation update... and maybe even a GitHub Action to publish it automatically (when it's actually acceptable quality). I'm going to push a few more documentation updates to this branch and it will merge in a little bit |
…ings) * COMPATIBILITY.txt - beautify it to make it clearer... might still have more updates after deployment * CHANGELOG.md - add in the line to deploy which I've been waiting for for a year...
|
Sheesh, that just doesn't look like 128 commits and a year of updates... but it's merged 😎 release coming shortly I appreciate both of you who have helped me along this journey! Can't thank you enough to help drive some good design decisions during the refactor! $ Merge made by the 'ort' strategy.
.github/workflows/lint-and-test.yml | 23 +-
.gitignore | 9 +
CHANGELOG.md | 14 +-
COMPATIBILITY.txt | 55 ++
LICENSE | 2 +-
README.rst | 195 +++++-
doc/index.rst | 8 -
{doc => docs}/.gitignore | 0
{doc => docs}/Makefile | 1 +
docs/make.bat | 35 +
{doc => docs/source}/conf.py | 59 +-
docs/source/index.rst | 39 ++
exiftool/__init__.py | 18 +-
exiftool/constants.py | 80 +++
exiftool/exceptions.py | 51 ++
exiftool/exiftool.py | 1283 +++++++++++++++++++++--------------
exiftool/experimental.py | 355 ++++++++++
exiftool/helper.py | 231 +++++++
mypy.ini | 2 +
scripts/README.txt | 7 +
scripts/flake8.bat | 21 +
scripts/flake8.ini | 9 +
scripts/flake8_requirements.txt | 6 +
scripts/mypy.bat | 16 +
scripts/mypy_reqiuirements.txt | 1 +
scripts/pytest.bat | 19 +
scripts/pytest_requirements.txt | 4 +
scripts/unittest.bat | 12 +
scripts/windows.coveragerc | 9 +
setup.cfg | 3 +
setup.py | 45 +-
tests/test_alpha.py | 241 +++++++
tests/test_exiftool.py | 434 +++++++-----
tests/test_helper.py | 154 +++++
34 files changed, 2671 insertions(+), 770 deletions(-)
create mode 100644 COMPATIBILITY.txt
delete mode 100644 doc/index.rst
rename {doc => docs}/.gitignore (100%)
rename {doc => docs}/Makefile (99%)
create mode 100644 docs/make.bat
rename {doc => docs/source}/conf.py (88%)
create mode 100644 docs/source/index.rst
create mode 100644 exiftool/constants.py
create mode 100644 exiftool/exceptions.py
create mode 100644 exiftool/experimental.py
create mode 100644 exiftool/helper.py
create mode 100644 mypy.ini
create mode 100644 scripts/README.txt
create mode 100644 scripts/flake8.bat
create mode 100644 scripts/flake8.ini
create mode 100644 scripts/flake8_requirements.txt
create mode 100644 scripts/mypy.bat
create mode 100644 scripts/mypy_reqiuirements.txt
create mode 100644 scripts/pytest.bat
create mode 100644 scripts/pytest_requirements.txt
create mode 100644 scripts/unittest.bat
create mode 100644 scripts/windows.coveragerc
create mode 100644 tests/test_alpha.py
create mode 100644 tests/test_helper.py
|
|
have to restore the branch as it's referred to in #39 |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Alright, this is still being developed, but I figured I'd get discussion on the code via a PR to myself. Therefore people can review code and make comments directly to code.
The base
ExifToolclass is pretty much complete. I'm planning to add logging functionality to help users debug what's going raw in/out of exiftool process, but that's about it.