Skip to content

Conversation

JukkaL
Copy link
Collaborator

@JukkaL JukkaL commented Oct 21, 2025

The test case needs to filter out quite a few things that are unpredictable.

@JukkaL JukkaL requested a review from ilevkivskyi October 21, 2025 10:44
Copy link
Member

@ilevkivskyi ilevkivskyi left a comment

Choose a reason for hiding this comment

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

LG, thanks!

@@ -1,4 +1,4 @@
"""Tool to convert mypy cache file to a JSON format (print to stdout).
"""Tool to convert binary mypy cache files (.ff) to JSON (.ff.json).
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
"""Tool to convert binary mypy cache files (.ff) to JSON (.ff.json).
"""Tool to convert binary mypy cache files (.ff) to JSON (.json).

Copy link
Member

Choose a reason for hiding this comment

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

Oh, I see you actually don't strip the original extension. I think it is better to do this, i.e. use .data.json or .meta.json for output files.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I did it this way so that there is no confusion between the "real" JSON cache files and the ones produced by the converter -- there's no guarantee that mypy will be able to read the output, since we don't guarantee that it stays in sync with the actual JSON format.

@github-actions
Copy link
Contributor

According to mypy_primer, this change doesn't affect type check results on a corpus of open source code. ✅

@JukkaL
Copy link
Collaborator Author

JukkaL commented Oct 21, 2025

Interesting, file hashes appear to be different on Windows. Maybe it's due to CRLF conversion. I will investigate -- maybe the test found a minor bug or at least inconsistency.

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.

2 participants