Skip to content

Fixed #973 updated min.py script with additional checks to scan files in the Stumpy repo that list package versions and confirm those versions align with the minimum supported versions#988

Closed
joehiggi1758 wants to merge 13 commits intostumpy-dev:mainfrom
joehiggi1758:update_min_py_with_additional_checks

Conversation

@joehiggi1758
Copy link
Contributor

@joehiggi1758 joehiggi1758 commented Jun 12, 2024

See #973

Pull Request Checklist

Below is a simple checklist but please do not hesitate to ask for assistance!

  • Fork, clone, and checkout the newest version of the code
  • Create a new branch
  • Make necessary code changes
  • Install black (i.e., python -m pip install black or conda install -c conda-forge black)
  • Install flake8 (i.e., python -m pip install flake8 or conda install -c conda-forge flake8)
  • Install pytest-cov (i.e., python -m pip install pytest-cov or conda install -c conda-forge pytest-cov)
  • Run black . in the root stumpy directory
  • Run flake8 . in the root stumpy directory
  • Run ./setup.sh && ./test.sh in the root stumpy directory
  • Reference a Github issue (and create one if one doesn't already exist)

…PY supports Python 3.8+ updated pypi.sh to reference step a) pyproject.toml instead of the decomissioned setup.py and ultimately scanned the full repository for any listings of outdated references to STUMPY supporting Python 3.7 (I was unable to find any)
@joehiggi1758 joehiggi1758 requested a review from seanlaw as a code owner June 12, 2024 13:39
@joehiggi1758 joehiggi1758 changed the title [#973] updated min.py script with additional checks to scan files in the Stumpy repo that list package versions and confirm those versions align with the minimum supported versions [TDAmeritrade#973] updated min.py script with additional checks to scan files in the Stumpy repo that list package versions and confirm those versions align with the minimum supported versions Jun 12, 2024
@joehiggi1758 joehiggi1758 changed the title [TDAmeritrade#973] updated min.py script with additional checks to scan files in the Stumpy repo that list package versions and confirm those versions align with the minimum supported versions [#973] updated min.py script with additional checks to scan files in the Stumpy repo that list package versions and confirm those versions align with the minimum supported versions Jun 12, 2024
@joehiggi1758
Copy link
Contributor Author

@seanlaw as a heads up I'm trying to figure out what went wrong here locally to where none of my tests are passing on the PR! I'll be pushing again once I figure this out!

Apologies for any confusion!

@seanlaw seanlaw changed the title [#973] updated min.py script with additional checks to scan files in the Stumpy repo that list package versions and confirm those versions align with the minimum supported versions Fixed #973 updated min.py script with additional checks to scan files in the Stumpy repo that list package versions and confirm those versions align with the minimum supported versions Jun 12, 2024
@joehiggi1758 joehiggi1758 force-pushed the update_min_py_with_additional_checks branch 2 times, most recently from 65ddc78 to 605e926 Compare June 13, 2024 15:45
@codecov
Copy link

codecov bot commented Jun 13, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 99.02%. Comparing base (f8f245c) to head (dc21488).
Report is 40 commits behind head on main.

Current head dc21488 differs from pull request most recent head 9082b76

Please upload reports for the commit 9082b76 to get more accurate results.

Additional details and impacted files
@@            Coverage Diff             @@
##             main     #988      +/-   ##
==========================================
+ Coverage   98.93%   99.02%   +0.09%     
==========================================
  Files          84       85       +1     
  Lines       14293    14413     +120     
==========================================
+ Hits        14141    14273     +132     
+ Misses        152      140      -12     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

… files in the Stumpy repo that list package versions and confirm those versions align with the minimum supported versions
@seanlaw
Copy link
Contributor

seanlaw commented Jun 20, 2024

@joehiggi1758 Can you please tell me all of the difference flavors of versioning styles that you've encountered in the files? In other words, what are all of the different types of regex you are using and what examples do they correspond to? I think there are ways for us to simplify things

For example, in the README.rst, I think we should change Python 3.8+ to `python>=3.8, which would make matching that text consistent with the others. But first, we need to have a clear view of all of the variations

@joehiggi1758
Copy link
Contributor Author

joehiggi1758 commented Jun 21, 2024

@seanlaw

Yes, more than happy to share!

  • The first regex f"{pkg}-version: [" is searching for the format "python-version: ['3.8']" for example, line 13 in github-actions.yml
  • The second regex rf"({pkg}|{pkg})\s*==?\s*['\"]?(\d+\.\d+(\.\d+)?)['\"]?") is searching for the format "requires-python = ">=3.8"" for example, line 8 in pyproject.toml
  • The third regex rf"({pkg})\s*>=\s*['\"]?(\d+\.\d+(\.\d+)?)['\"]?" is searching for the format "python>=3.8" for example, line 5 in environment.yml
  • The fourth regex rf"::\s*{pkg}\s*::\s*['\"]?(\d+\.\d+(\.\d+)?)['\"]?" is searching for the format "Programming Language :: Python :: 3.8" for example, line 26 in pyproject.toml
  • The fifth regex rf"{pkg}\s+[a-z]+\s+(\d+\.\d+(\.\d+)?)" was left in on accident and should be removed
  • The sixth format is "STUMPY supports `Python...." for example, line 328 in README.rst

@seanlaw
Copy link
Contributor

seanlaw commented Jun 21, 2024

@joehiggi1758 If we change the README to python>=3.8, then do you think (building off of your work) something like the following minimum-reproducible-example might suffice?

import re

fnames = [
    "pyproject.toml",
    "requirements.txt",
    "environment.yml",
    ".github/workflows/github-actions.yml",
    "README.rst",
]

MIN_PYTHON = "2.7"
MIN_NUMBA = "0.1"
MIN_NUMPY = "0.1"
MIN_SCIPY = "0.1"

pkgs = {
    "numpy": MIN_NUMPY,
    "scipy": MIN_SCIPY,
    "numba": MIN_NUMBA,
    "python": MIN_PYTHON,
    "python-version": MIN_PYTHON,
}

for pkg_name, pkg_version in pkgs.items():
    for fname in fnames:
        with open(fname, "r") as file:
            for line_num, line in enumerate(file, start=1):
                l = line.strip().replace(" ", "").lower()
                matches = re.search(
                    rf"""
                                {pkg_name}  # Package name
                                [=><:"\'\[\]]+  # Zero or more special characters
                                (\d+\.\d+[\.0-9]*)  # Capture "version" in `matches`
                                """,
                    l,
                    re.VERBOSE,  # Ignores all whitespace in pattern
                )
                if matches is not None:
                    version = matches.groups()[0]
                    if version != pkg_version:
                        print(
                            f'Package Mismatch Found: "{pkg_name}" "{version}" in {fname}:{line_num} '
                        )

Does this miss any edge cases?

@joehiggi1758
Copy link
Contributor Author

joehiggi1758 commented Jun 21, 2024

@seanlaw

I do believe that would suffice, I can start reworking my local min.py - thanks for throwing that together! However I believe that regex misses the following edge cases (I tested using regex101.com)...

  • Programming Language :: Python :: 3.8
  • STUMPY supports Python 3.8
  • python-version: ['3.8']
  • requires-python = ">=3.8"
  • numba>=0.55.2

I will work on trying to get a regex to match all edge cases!

@seanlaw
Copy link
Contributor

seanlaw commented Jun 21, 2024

However I believe that regex misses the following edge cases (I tested using regex101.com)...

Hmm, can you tell me which lines those correspond to? I am seeing this output when I run the code:

Package Mismatch Found: "numpy" "1.21" in pyproject.toml:35
Package Mismatch Found: "numpy" "1.21" in requirements.txt:1
Package Mismatch Found: "numpy" "1.21" in environment.yml:6
Package Mismatch Found: "scipy" "1.10" in pyproject.toml:36
Package Mismatch Found: "scipy" "1.10" in requirements.txt:2
Package Mismatch Found: "scipy" "1.10" in environment.yml:7
Package Mismatch Found: "numba" "0.57.1" in pyproject.toml:37
Package Mismatch Found: "numba" "0.57.1" in requirements.txt:3
Package Mismatch Found: "numba" "0.57.1" in environment.yml:8
Package Mismatch Found: "python" "3.8" in pyproject.toml:8
Package Mismatch Found: "python" "3.8" in pyproject.toml:26
Package Mismatch Found: "python" "3.8" in environment.yml:5
Package Mismatch Found: "python" "3.8" in README.rst:328
Package Mismatch Found: "python-version" "3.8" in .github/workflows/github-actions.yml:13
Package Mismatch Found: "python-version" "3.8" in .github/workflows/github-actions.yml:66
Package Mismatch Found: "python-version" "3.8" in .github/workflows/github-actions.yml:113

The file and line number are printed at the end of the line as file:line_num

In case you missed it, in addition to the regex, I also did l = line.strip().replace(" ", "").lower() and it appears to be enough to cover all cases. Otherwise, please let me know which lines in which file are missed.

@joehiggi1758
Copy link
Contributor Author

joehiggi1758 commented Jun 21, 2024

@seanlaw

I see, yes you're correct I overlooked that strip and replace line! All edge cases look to be accounted for!

seanlaw and others added 8 commits June 21, 2024 22:23
…ted functions (stumpy-dev#991)

* Add param copy to preprocess-related functions

* move new param to the end of the list of existing parameters to ensure backward compatibility

* fix format
… files in the Stumpy repo that list package versions and confirm those versions align with the minimum supported versions
… files in the Stumpy repo that list package versions and confirm those versions align with the minimum supported versions
@review-notebook-app
Copy link

Check out this pull request on  ReviewNB

See visual diffs & provide feedback on Jupyter Notebooks.


Powered by ReviewNB

@joehiggi1758 joehiggi1758 deleted the update_min_py_with_additional_checks branch June 22, 2024 02:35
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.

3 participants