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

Handle version specifiers #190

Merged
merged 35 commits into from
Nov 10, 2023
Merged

Handle version specifiers #190

merged 35 commits into from
Nov 10, 2023

Conversation

CasperWA
Copy link
Collaborator

@CasperWA CasperWA commented Oct 9, 2023

Fixes #141

This is a different approach to fixing #141 from #181, in that instead of creating a whole new class (SemanticVersionRange) to define the version range specified by the version specifier set of a dependency, it utilizes the packaging package to parse the version specifier set and generate "ignore" rules to utilize the existing logic for handling "ignore" rules.

In this way, it's more of a "minimum implementation" and should be easier and more straight forward. However, what may really be happening, is simply just making the reading and understanding of the "ignore" rules implementation the point at which one may struggle to understand the implementation.
But again, since this logic already exists, this does not add additional complexity (which cannot be said of the solution presented in #181).

In addition to parsing the dependency-specific version specifier set as "ignore" rules, support for the python_version marker is also implemented, although be it in a limited capacity - mainly due to not over-complicating the logic further.

Also, the utils.py file has been split up into it's separate parts and moved into dedicated files under a new utils folder - effectively changing the ci-cd package's Python API.

Finally, some formatting options have been added when printing to the console. These options include colorization and general formatting (bold, italic, etc.). Note, these are only displayed in any given terminal if the terminal supports it.
These options have only been implemented for the update-deps task, since this is the only one being touched properly by this PR.

Note, the Python API changes mentioned above has also resulted in a change in the test file structure.
Because all utility functions previously found in ci_cd.tasks.update_deps have been moved to the new ci_cd.utils.versions module, some tests have also been moved file, accordingly.

Further suggested improvements for future PRs can include:

  • Check if the "ignore" rules logic can be updated with the usage of the packaging package.

Things to do prior to this PR being ready for review:

  • Add tests (according to code coverage report)
  • Ignore warnings if self-references are made in optional dependencies

Split up utils.py into a separate "utils" module.
Depending on each dependency's set of specifiers, a list of on-the-fly
ignore rules are created and added to the otherwise user-provided ignore
rules.
This is a way of using the existing implementation of "ignore rules" to
more intelligently suggest an updated set of version specifiers for a
dependency based on the latest version retrieved from 'pip index
versions'.
- Instead of completely skipping when URL or non-versioned dependency is
found, we "regenerate" the dependency in order to ensure consistent
formatting.
- Properly continue on from current dependency when it is found that the
latest version is already used in the pyproject.toml file.
- Don't fail if a current version cannot be found based on '==', '>=',
or '~=' operators. Instead use '0.0.0'.
- Always replace "-quotations with '-quotations for the updated
dependency line when updating the pyproject.toml file.
Only not-equal operator (!=) is relevant.

Also, update tests folder API to reflect the updated Python API,
including the moved utility functions.
Update tests to better reflect edge cases.
The old functionality is kept in to be re-implemented for other
frameworks and/or choose it in a future toggle option.
Instead of through 'pip._vendor.packaging'.
Add as core dependency.
Only having a single python_version marker is supported, furthermore,
having any other markers are not respected by this implementation.
Instead of having this logic embedded in 'get_min_max_py_version()'.
@codecov
Copy link

codecov bot commented Oct 9, 2023

Codecov Report

Merging #190 (6c71755) into main (2cf64e4) will increase coverage by 7.85%.
The diff coverage is 90.90%.

@@            Coverage Diff             @@
##             main     #190      +/-   ##
==========================================
+ Coverage   72.09%   79.95%   +7.85%     
==========================================
  Files           9       12       +3     
  Lines         577      828     +251     
==========================================
+ Hits          416      662     +246     
- Misses        161      166       +5     
Files Coverage Δ
ci_cd/__init__.py 100.00% <100.00%> (ø)
ci_cd/exceptions.py 100.00% <100.00%> (ø)
ci_cd/tasks/api_reference_docs.py 64.33% <100.00%> (-0.25%) ⬇️
ci_cd/tasks/setver.py 22.00% <100.00%> (-1.53%) ⬇️
ci_cd/tasks/update_deps.py 100.00% <100.00%> (+17.30%) ⬆️
ci_cd/utils/__init__.py 100.00% <100.00%> (ø)
ci_cd/utils/console_printing.py 100.00% <100.00%> (ø)
ci_cd/utils/file_io.py 85.71% <85.71%> (ø)
ci_cd/utils/versions.py 86.76% <86.76%> (ø)

@CasperWA CasperWA marked this pull request as draft October 9, 2023 11:16
Add pyupgrade as a pre-commit hook with --py37-plus arg.
Run and upgrade all files accordingly.

Configure mypy to check typing expecting Python 3.7.
Check that e.g. using the unsupported '===' operator raises an
exception as expected.
@CasperWA CasperWA marked this pull request as ready for review November 9, 2023 07:13
Copy link
Contributor

@daniel-sintef daniel-sintef left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It looks OK, I think we should keep reviews under 375 LOC per 45 minute session https://smartbear.com/learn/code-review/best-practices-for-peer-code-review/

@CasperWA CasperWA merged commit 67d5ac4 into main Nov 10, 2023
18 checks passed
@CasperWA CasperWA deleted the cwa/fix-141-utilize-ignore branch November 10, 2023 14:13
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.

Ensure version dependency ranges are respected when updating
2 participants