Multiple Development (v6 betas) re-joined - #361
Conversation
…ile the public API is still WIP from (- WIP #316 -) Also includes other minor improvements like normalizing the module path (e.g., pip-licenses -> piplicenses)
* Also added lots of comments about direction of API re-exports that are needed for compatibility with older versions * Also fixed a bunch more F401 lint issues * Also reorganized a bit more * Also cleaned up file headers a little * And other trivial fixes all over
* fixes for breaking-changes to tests will come later * This development version also contributes to GHI #81
reactive-firewall
left a comment
There was a problem hiding this comment.
👍 LGTM;
Note
with caveat: I am the project owner/admin and was this PR's author and I self-reviewed.
Additional Comments: see many in-line comments for more details.
| _ignore_packages_map, | ||
| ): | ||
| if _pre_processed_name in _mapping: | ||
| if not "6.0" in __version__: |
There was a problem hiding this comment.
🧹 NIT: This will need to be removed as "always true" in the future.
- As part of CHORE: draft documentation for v6 series changes/migration #349 these mapping changes need documented in detail.
| booleans = { | ||
| "summary", | ||
| "with_system", | ||
| "with_authors", | ||
| "with_maintainers", | ||
| "with_urls", | ||
| "with_description", | ||
| "with_license_files", | ||
| "with_notice_file", | ||
| "with_notice_files", | ||
| "with_other_files", | ||
| "without_license_paths", | ||
| "without_file_paths", | ||
| "filter_strings", | ||
| "partial_match", | ||
| "without_version", | ||
| } |
There was a problem hiding this comment.
🧹 NIT: consider further modularizing this to an internal function for future extensibility
e.g.
def _get_canonical_boolean_feilds(self) -> set[str]:
return {
"summary",
"with_system",
"with_authors",
"with_maintainers",
"with_urls",
"with_description",
"with_license_files",
"with_notice_file",
"with_notice_files",
"with_other_files",
"without_license_paths",
"without_file_paths",
"filter_strings",
"partial_match",
"without_version",
}| booleans = { | |
| "summary", | |
| "with_system", | |
| "with_authors", | |
| "with_maintainers", | |
| "with_urls", | |
| "with_description", | |
| "with_license_files", | |
| "with_notice_file", | |
| "with_notice_files", | |
| "with_other_files", | |
| "without_license_paths", | |
| "without_file_paths", | |
| "filter_strings", | |
| "partial_match", | |
| "without_version", | |
| } | |
| booleans = self._get_canonical_boolean_feilds() |
- As part of CHORE: draft documentation for v6 series changes/migration #349 the final status of this (is/is-not a helper function) should be clarified before release.
| "partial_match", | ||
| "without_version", | ||
| } | ||
| sets = {"ignore_packages", "packages"} |
There was a problem hiding this comment.
🧹 NIT: Similarly consider further modularizing this to an internal function for future extensibility
e.g.
def _get_canonical_multi_feilds(self) -> set[str]:
return {
"ignore_packages",
"packages",
# etc.
}| sets = {"ignore_packages", "packages"} | |
| sets = self._get_canonical_multi_feilds() |
- As part of CHORE: draft documentation for v6 series changes/migration #349 the final status of this (is/is-not a helper function) should be clarified before release.
| def packages_set(self, value: Union[Iterable[str], None]) -> None: | ||
| self.packages = self._normalize_to_set(value) | ||
|
|
||
| if "6.1" in __version__: |
There was a problem hiding this comment.
🧹 NIT: This will need to be removed as "always true" in the future.
- As part of CHORE: draft documentation for v6 series changes/migration #349 these mapping changes need documented in detail.
| # TODO: [GHI-394](https://github.com/raimon49/pip-licenses/issues/349) | ||
| LICENSE_BY_OTHER_FILE_PATTERN = r"[Aa][Uu][Tt][Hh][Oo][Rr][Ss].*|[Cc][Oo][Pp][Yy][Ii][Nn][Gg].*|[Ll][Ee][Gg][Aa][Ll].*" |
There was a problem hiding this comment.
- As part of CHORE: draft documentation for v6 series changes/migration #349 this new "other files" needs documented in more detail.
| "licensefile": legacy_info["license_file"], # DEPRECIATED in v6.0+ | ||
| "licensetext": legacy_info["license_text"], # DEPRECIATED in v6.0+ | ||
| "noticefile": legacy_info["notice_file"], # DEPRECIATED in v6.0+ | ||
| "noticetext": legacy_info["notice_text"], # DEPRECIATED in v6.0+ | ||
| "otherfile": legacy_info["author_file"], # DEPRECIATED in v6.0+ | ||
| "othertext": legacy_info["author_text"], # DEPRECIATED in v6.0+ |
There was a problem hiding this comment.
- As part of CHORE: draft documentation for v6 series changes/migration #349 this deprecation needs documented in detail.
| FILE_MISSING = "" | ||
| return { | ||
| "license_file": license_file or LICENSE_UNKNOWN, | ||
| "license_file": license_file or FILE_MISSING, |
There was a problem hiding this comment.
- As part of CHORE: draft documentation for v6 series changes/migration #349 this change needs documented in more detail.
| (SUBJECT TO CHANGE IN NEXT RELEASE) | ||
|
|
||
| #### From ALPHA (already backported) | ||
| * Align with [PEP-749](https://peps.python.org/pep-0749) and deprecated use of `typing.TYPE_CHECKING` conditions |
There was a problem hiding this comment.
- As part of CHORE: draft documentation for v6 series changes/migration #349 this deprecation needs documented in detail.
| * Align with [PEP-749](https://peps.python.org/pep-0749) and deprecated use of `typing.TYPE_CHECKING` conditions | ||
| * Fixed multiple typographical errors | ||
| * Refactored most of the codebase to be more modular (WIP GHI #316) | ||
|
|
There was a problem hiding this comment.
This is missing a lot of change details still
Patch Notes
This patch brings the development branch another big step closer to the major v6-series release. Most notably, the major codebase refactoring for GHI-316 (modularization).
Responsibilities are separated (core scanning vs CLI parsing vs formatting), which improves maintainability and testability.
Change Summary:
piplicenses.pyinto a packagepiplicenseswith submodules:core.py: package discovery, metadata extraction, file text readingcli/: configuration dataclass & argparse glue, pseudo-enum helpersoutput/: PrettyTable subclasses, table builders, consoles helpersconstants.py,sorting.py,tomli_bridge.py, plus__main__.pyand package-level__init__.pythat re-exports compatibility API.This version also includes these changes:
typing.TYPE_CHECKINGconditionsflowchart TD %% Single-file to package split A["piplicenses.py (single file)"] --> B["piplicenses/__init__.py"] A --> C["piplicenses/core.py"] A --> D["piplicenses/cli/*.py"] A --> E["piplicenses/output/*.py"] A --> F["piplicanses/ (other modules: etc.)"] %% Optional: make the intended package structure visually explicit subgraph PKG["piplicenses/ package"] direction TB B1["__init__.py"] C1["core.py"] D1["cli/ (multiple .py files)"] E1["output/ (multiple .py files)"] F1["etc. (additional modules)"] B1 -->|exports/API| B C1 -->|core logic| C D1 -->|command-line entrypoints| D E1 -->|formatters/tables| E F1 -->|additional code| F endImpacted GHIs:
pip-licenses/#316 (W.I.P.)Included & Superseded PR/MRs
Important
CI/CD Impacted by Breaking changes 🚧
CI/CD is expected to fail with these (and related) changes, because of:
piplicenses._handle_multiple_value_field) has moved (and will not be part of a publicly exposed API, and will need to be imported directly for testing). The known fix is to appendfrom piplicenses.output.tables import _handle_multiple_value_fieldto the imports intest_piplicenses.pyopen(previously applied generally inpiplicenses) to focus only on patchingopenused for writing operations selectively (namely those for file serialization, e.g. currentlypiplicenses.output.consoles) by replacing:sys.exit(...). While subtle, this change, is still known to have rendered the mocking of system exits in failure-validation testing. The known fix is to handle and evenasserttheSystemExitConfigurationdata class (W.I.P.) some tests that relied on a stable order when comparing multi-value fields need to be sorted in testing to remain reproducible. Namely the known fix is to sort parsedsetvalues when comparing tolistsvia thesortedPython builtin.Otherwise, this patch passes the same tests as #351 (and thus supersedes that PR).