-
Notifications
You must be signed in to change notification settings - Fork 20
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
add numpy 2 hotfix to main #227
Changes from 35 commits
7f0f367
39ec561
013d769
b011dcc
e38027f
67417d8
0a93cab
d0d073b
d35eab8
513846a
9e1ab7b
b0fd63e
71306c3
ce7e20b
0863898
c979b12
881cf75
18e3c4d
e3cb04b
86c54a1
0027dab
571832b
34f2d32
fec2680
ed2659a
4d654e5
c26a1fd
71e30cd
d75edb5
411223c
6716a8f
8dd4c11
c896b1b
d2ab4c9
2d59ddb
7f237ee
c1b7795
7639c1a
c09fa2f
4326c27
1eb8cd5
e5bb64f
3b6625c
c54945f
36222d0
4d8e248
b6595af
4d468b1
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change | ||||||
---|---|---|---|---|---|---|---|---|
|
@@ -7,10 +7,21 @@ | |||||||
import sys | ||||||||
from collections import defaultdict | ||||||||
from os.path import dirname, isdir, isfile, join | ||||||||
|
||||||||
from conda.models.version import VersionOrder | ||||||||
|
||||||||
import csv | ||||||||
import requests | ||||||||
import logging | ||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I don't think it is a good idea to add in logging into this script. This script is feed into a conda-index process. |
||||||||
|
||||||||
# Global dictionary to store data for CSV output | ||||||||
csv_data = defaultdict(list) | ||||||||
|
||||||||
# Configure the logging | ||||||||
logging.basicConfig(level=logging.DEBUG, | ||||||||
format='%(asctime)s - %(name)s - %(levelname)s - %(message)s', | ||||||||
handlers=[logging.FileHandler('hotfixes.log', mode='w'), | ||||||||
logging.StreamHandler()]) | ||||||||
# Create a logger object | ||||||||
logger = logging.getLogger(__name__) | ||||||||
|
||||||||
CHANNEL_NAME = "main" | ||||||||
CHANNEL_ALIAS = "https://repo.anaconda.com/pkgs" | ||||||||
|
@@ -261,6 +272,95 @@ | |||||||
] | ||||||||
|
||||||||
|
||||||||
def load_numpy2_changes(): | ||||||||
try: | ||||||||
with open('numpy2_patch.json', 'r') as f: | ||||||||
return json.load(f) | ||||||||
except FileNotFoundError: | ||||||||
logger.error("numpy2_patch.json not found. Aborting hotfixes.") | ||||||||
sys.exit(1) | ||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I would just remove this block and use my oneliner below. |
||||||||
|
||||||||
|
||||||||
NUMPY_2_CHANGES = load_numpy2_changes() | ||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
Of course, add in |
||||||||
|
||||||||
|
||||||||
def apply_numpy2_changes(record, subdir, filename): | ||||||||
""" | ||||||||
Applies predefined numpy changes to a record based on its directory and filename. | ||||||||
|
||||||||
Parameters: | ||||||||
- record: The record to update. | ||||||||
- subdir: The subdirectory of the record. | ||||||||
- filename: The filename of the record. | ||||||||
""" | ||||||||
if subdir not in NUMPY_2_CHANGES or filename not in NUMPY_2_CHANGES[subdir]: | ||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
Small optimization. |
||||||||
return | ||||||||
changes = NUMPY_2_CHANGES[subdir][filename] | ||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. You can then refer to subdir_changes from above. |
||||||||
for change in changes: | ||||||||
depends = _get_dependency_list(record, change['type']) | ||||||||
if depends is None: | ||||||||
continue | ||||||||
_apply_changes_to_dependencies(depends, change, record, filename, 'type') | ||||||||
|
||||||||
|
||||||||
def _get_dependency_list(record, change_type): | ||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I don't think this warrants its own function. If you store "depends" and "constrains" directly in the numpy2_patch.json file, the replace_dep line becomes:
Your process already did all the cleaning so we shouldn't have any weird third cases. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This also gets rid of the unnecessary for loop. |
||||||||
""" | ||||||||
Returns the appropriate dependency list based on the change type. | ||||||||
|
||||||||
Parameters: | ||||||||
- record (dict): The record containing dependency information. | ||||||||
- change_type (str): The type of change ('dep' for dependencies, 'constr' for constraints). | ||||||||
|
||||||||
Returns: | ||||||||
- list: The list of dependencies or constraints based on the change type, None if the change type is unrecognized. | ||||||||
""" | ||||||||
if change_type == 'dep': | ||||||||
return record['depends'] | ||||||||
elif change_type == 'constr': | ||||||||
return record.get('constrains', []) | ||||||||
return None | ||||||||
|
||||||||
|
||||||||
def _apply_changes_to_dependencies(depends, change, record, filename, sort_type='reason'): | ||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||||||
""" | ||||||||
Applies changes to dependencies and logs the changes. | ||||||||
|
||||||||
Parameters: | ||||||||
- depends (list): The list of dependencies to be modified. | ||||||||
- change (dict): A dict containing the original dependency, the updated dependency, the reason for the change. | ||||||||
- record (dict): The record to which the changes apply. | ||||||||
- filename (str): The name of the file being processed. | ||||||||
- sort_type (str, optional): The key in the 'change' dictionary to sort the CSV data by. Defaults to 'reason'. | ||||||||
""" | ||||||||
for i, dep in enumerate(depends): | ||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
replace_dep is the standard usage for replacing deps in main.py. |
||||||||
if dep == change['original']: | ||||||||
depends[i] = change['updated'] | ||||||||
if change['reason'] == 'Upper bound added': | ||||||||
logger.info(f"Applied numpy change for {filename}: {change['original']} -> {change['updated']}") | ||||||||
# Add to csv_data for later CSV export | ||||||||
csv_data[change[sort_type]].append([ | ||||||||
record['name'], record['version'], record['build'], | ||||||||
record['build_number'], change['original'], | ||||||||
change['updated'], change['reason'] | ||||||||
]) | ||||||||
|
||||||||
|
||||||||
def write_csv(): | ||||||||
""" | ||||||||
Writes update data to CSV files in the 'updates' directory. | ||||||||
""" | ||||||||
if not os.path.exists("updates"): | ||||||||
os.makedirs("updates") | ||||||||
|
||||||||
for issue_type, data in csv_data.items(): | ||||||||
with open(f"updates/{issue_type}_numpy2_updates.csv", 'w', newline='') as csvfile: | ||||||||
csv.writer(csvfile).writerow(['Package', 'Version', | ||||||||
'Build', 'Build Number', | ||||||||
'Original Dependency', 'Updated Dependency', | ||||||||
'Reason']) | ||||||||
csv.writer(csvfile).writerows(data) | ||||||||
|
||||||||
|
||||||||
def _replace_vc_features_with_vc_pkg_deps(name, record, depends): | ||||||||
python_vc_deps = { | ||||||||
"2.6": "vc 9.*", | ||||||||
|
@@ -671,6 +771,9 @@ def patch_record_in_place(fn, record, subdir): | |||||||
depends[i] = depends[i].replace(">=1.21.5,", ">=1.21.2,") | ||||||||
break | ||||||||
|
||||||||
if NUMPY_2_CHANGES is not {}: | ||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
Python truthiness. Depending on how you generated the patch. We should either apply all the numpy patches at the very beginning or at the very end. There are other places in main where numpy is touched and gets an upper bound. We don't want to overwrite them or redo them. |
||||||||
apply_numpy2_changes(record, subdir, fn) | ||||||||
|
||||||||
########### | ||||||||
# pytorch # | ||||||||
########### | ||||||||
|
@@ -734,7 +837,6 @@ def patch_record_in_place(fn, record, subdir): | |||||||
###################### | ||||||||
# scipy dependencies # | ||||||||
###################### | ||||||||
|
||||||||
# scipy 1.8 and 1.9 introduce breaking API changes impacting these packages | ||||||||
if name == "theano": | ||||||||
if version in ["1.0.4", "1.0.5"]: | ||||||||
|
@@ -969,7 +1071,6 @@ def patch_record_in_place(fn, record, subdir): | |||||||
|
||||||||
# kealib 1.4.8 changed sonames, add new upper bound to existing packages | ||||||||
replace_dep(depends, "kealib >=1.4.7,<1.5.0a0", "kealib >=1.4.7,<1.4.8.0a0") | ||||||||
|
||||||||
# Other broad replacements | ||||||||
for i, dep in enumerate(depends): | ||||||||
# glib is compatible up to the major version | ||||||||
|
@@ -1534,6 +1635,8 @@ def do_hotfixes(base_dir): | |||||||
def main(): | ||||||||
base_dir = join(dirname(__file__), CHANNEL_NAME) | ||||||||
do_hotfixes(base_dir) | ||||||||
if NUMPY_2_CHANGES != {}: | ||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We shouldn't be creating csv files as a by product of the hotfixing process. |
||||||||
write_csv() | ||||||||
|
||||||||
|
||||||||
if __name__ == "__main__": | ||||||||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks like you need to update this to reflect current code changes.