Skip to content
Snippets Groups Projects

Add ZIP archives support

Closed Add ZIP archives support
smagnin/mat2:archive into master
2 unresolved threads
Closed Simon Magnin requested to merge smagnin/mat2:archive into master
2 unresolved threads

I create this MR to have your help. There are some issues with the CI. Can you help me to correct them ?

Merge request reports

Pipeline #19532 failed

Stage: linting
Stage: test

Pipeline failed for dc194d59 on smagnin:archive

Closed by jvoisinjvoisin 6 years ago (Oct 25, 2018 11:30am UTC)

Loading

Activity

Filter activity
  • Approvals
  • Assignees & reviewers
  • Comments (from bots)
  • Comments (from users)
  • Commits & branches
  • Edits
  • Labels
  • Lock status
  • Mentions
  • Merge request status
  • Tracking
  • jvoisin
    jvoisin @jvoisin
    Owner

    well, the CI messages are pretty clear:

    $ pylint3 --extension-pkg-whitelist=cairo,gi ./libmat2 ./mat2
************* Module libmat2.archive
W: 53, 4: Static method with 'self' as first argument (bad-staticmethod-argument)
R: 74, 4: Too many branches (15/12) (too-many-branches)
************* Module mat2
W: 91,52: Using possibly undefined loop variable 'k' (undefined-loop-variable)
  • jvoisin
    jvoisin @jvoisin started a thread on the diff
libmat2/archive.py
49 50 return zipinfo
50 51
51 52 @staticmethod
52 def _get_zipinfo_meta(zipinfo: zipfile.ZipInfo) -> Dict[str, str]:
53 def _get_zipinfo_meta(self, zipinfo: zipfile.ZipInfo) -> Dict[str, str]:
  • jvoisin
    jvoisin @jvoisin started a thread on the diff
    • libmat2/archive.py
    64 65 if zipinfo.date_time != (1980, 1, 1, 0, 0, 0):
    65 66 metadata['date_time'] = str(datetime.datetime(*zipinfo.date_time))
    66 67
    68 ret = self._parse_files()
    69 metadata_files = ret[0]
    70 for name, _ in metadata_files.items():
    71 metadata[name] = metadata_files
    67 72 return metadata
    68 73
    69 def remove_all(self) -> bool:
    70 # pylint: disable=too-many-branches
    71
    74 def _parse_files(self) -> tuple:
    75 metadata = {} # type: dict
    76 caller = sys._getframe(1).f_code.co_name
    • jvoisin
      jvoisin @jvoisin
      Owner

      Ouch, please don't do that. Feel free to copy/paste code instead of using this kind of black magic

      Edited by jvoisin
    • Simon Magnin
      Simon Magnin @smagnin
      Author Contributor

      You didn't say to avoid c/c code ? So you prefer to have twice the code to remove and show metadata?

      Edited by Simon Magnin
    • jvoisin
      jvoisin @jvoisin
      Owner

      Absolutely :)

      We can always refactor later, but if the code is too, well, magic, refactoring can become pretty hard.

      Side notes:

      • You're not really supposed to call methods prefixed by _ outside of the class implementing them
      • Runtime introspection of the callstack isn't something that should be done in normal Python programs. This feature is mostly here to build debuggers.
      • Runtime introspection makes it harder to reason about program; static analyzers are already struggling with regular Python, no need to make their job harder :D
      Edited by jvoisin
    • Please register or sign in to reply
  • jvoisin
    jvoisin @jvoisin
    Owner

    Aw, you did a single MR for both archive support, and recursive metadata :/ What about only adding zip support, and then, in an other PR, recursive metadata?

    Edited by jvoisin
  • Simon Magnin
    Simon Magnin @smagnin
    Author Contributor

    Recursive metadata is only a problem of ZIP support no ?

  • jvoisin
    jvoisin @jvoisin
    Owner

    Not really: office files too, like an office document or a pdf, with an embedded png. Likely video too at some point. Some audio as well, like FLAC, can embed covers.

  • Simon Magnin added 1 commit

    added 1 commit

    Compare with previous version

  • Simon Magnin added 1 commit

    added 1 commit

    Compare with previous version

  • Simon Magnin
    Simon Magnin @smagnin
    Author Contributor

    Ok I will revert the metadata printing recursively and do it in another MR.

  • Simon Magnin added 1 commit

    added 1 commit

    Compare with previous version

  • Simon Magnin added 1 commit

    added 1 commit

    Compare with previous version

  • jvoisin
    jvoisin @jvoisin
    Owner

    Implemented in 3a070b0a, feel free to add more archive formats ;)

  • jvoisin closed

    closed

    • Please register or sign in to reply