Conversation
grische
left a comment
There was a problem hiding this comment.
Also, please clean up your commit history.
As we are having a lot of back and forth here, you can also ask some modern LLM for a review of your changes first and address those before updating the PR.
Claude Free tier (from claude.ai) should already be sufficient enough to give you some good feedback.
| # ruff: noqa: S310 | ||
| with urllib.request.urlopen(fileinfo["url"]) as inp, open(target, "wb") as out: |
There was a problem hiding this comment.
| # ruff: noqa: S310 | |
| with urllib.request.urlopen(fileinfo["url"]) as inp, open(target, "wb") as out: | |
| with urllib.request.urlopen(fileinfo["url"]) as inp, open(target, "wb") as out: # ruff: noqa: S310 |
Otherwise the noqa applies to the rest of the file.
| # At least one of the tags must match ALL of our environments | ||
| for tag in fileinfo["tags"]: | ||
| (intrp_name, intrp_ver) = parse_interpreter(tag.interpreter) | ||
| intrp_name, intrp_ver = parse_interpreter(tag.interpreter) |
There was a problem hiding this comment.
This change is unrelated to the PR and should be not part of the PR or at least in a separate commit explaining why
| super().__setitem__(key, value) | ||
|
|
||
|
|
||
| def download_req(index_url: str, req_name: str) -> tuple[dict, str]: |
There was a problem hiding this comment.
The name of the function is oddly chosen. Either change the function to be a generic function that can be reused (hence has a reason to be in utils.py) or move it into the place where it is meant to be and name it more specifically.
| self[key].extend(value) | ||
| else: | ||
| super().__setitem__(key, value) | ||
|
|
There was a problem hiding this comment.
This function is OK to have, but right now it is doing code duplication within metadata.py.
Either use the function everywhere, or not create a function at all.
use "Accept-Encoding": "gzip" to reduce traffic