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

Improve files module #303

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

Improve files module #303

wants to merge 4 commits into from

Conversation

isHarryh
Copy link
Contributor

Summary

This PR makes code quality improvements to the files module, such as type hints and typo corrections.

Commit Details

1. fix: better type hint in EndianBinaryReader/Writer

In EndianBinaryReader, use -> Tuple[xxx, ...] instead of -> Tuple[xxx], where the latter one is a common mistake.

In EndianBinaryWriter, use value: Sequence instead of value: list because some invocations pass a tuple argument instead of list.

2. refactor(files): use assertion for version-specific assignment

Use assert xxx is not None before value assignment.

This is also the practice adopted by MeshHelper and other classes, which can correctly satisfy the NoneType checking.

3. chore(files): fix typo and remove legacy code

In File class:

  • class File(object) can be simplified to class File (Python 3).
  • self.environment = self.environment = ... causes a duplicated self-assignment.

In ContainerHelper, method __or__ cannot be used anymore since 5ac0a62 changed the constructor of this class.

4. fix(files): overall type hint corrections

Correct the type hints in the files module.

Refactors BundleFile.get_version_tuple:

  • Uses (next(...), next(...), next(...)) instead of tuple(map(...)) to respect the returned type -> Tuple[int, int, int].
  • Prevents AttributeError by validating the regex match before accessing groups.

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.

1 participant