Skip to content

Conversation

@Eddy114514
Copy link
Collaborator

add warning for random.random() that in python 3 its string representation is longer (~18 digit) than python 2 (~14 digit).

as following:
image
image
image

@ltratt
Copy link
Member

ltratt commented Oct 22, 2025

Please force push a rebase against master before review.

@Eddy114514 Eddy114514 enabled auto-merge October 22, 2025 14:29
@Eddy114514 Eddy114514 disabled auto-merge October 22, 2025 14:29
@Eddy114514
Copy link
Collaborator Author

Please force push a rebase against master before review.

@ltratt , resolved, thanks for pointing out.

@ltratt
Copy link
Member

ltratt commented Oct 23, 2025

Please squash.

@ltratt
Copy link
Member

ltratt commented Oct 23, 2025

Hmm, actually, maybe I'm wrong.

Isn't this warning going to be annoying? Even if someone has updated their code, they're still going to get the "change your code" warning AFAICS?

@Eddy114514
Copy link
Collaborator Author

Hmm, actually, maybe I'm wrong.

Isn't this warning going to be annoying? Even if someone has updated their code, they're still going to get the "change your code" warning AFAICS?

@ltratt I think you are right about the warning behavior of my implementation. I think it is generally hard to detect whether user change their code within the warning.

Since the warning is about length of random.random(), maybe I should put warning inside of len() or __str__ method of random.random (if there is one) such that it is less frequent. But I think it still can't detect whether user change their code or not.

@ltratt
Copy link
Member

ltratt commented Oct 23, 2025

What one would need to do is track the string output by random.random and then see if someone does something "bad" on it (e.g. len). There are various ways one could do this.

For example, @nanjekyejoannah can fill you in on the bstate idea we've long been working towards. A variant of that would work here: one could (say) add a bool to PyStr that is true if the string was produced by random.random. This is pretty heavy-weight, admittedly.

Another way would be to return a "special" string-like object from random and intercept calls to it. I'm not sure how hard that might be.

@Eddy114514
Copy link
Collaborator Author

What one would need to do is track the string output by random.random and then see if someone does something "bad" on it (e.g. len). There are various ways one could do this.

For example, @nanjekyejoannah can fill you in on the bstate idea we've long been working towards. A variant of that would work here: one could (say) add a bool to PyStr that is true if the string was produced by random.random. This is pretty heavy-weight, admittedly.

Another way would be to return a "special" string-like object from random and intercept calls to it. I'm not sure how hard that might be.

@ltratt Thank you for your suggestions on improving my implementation, I will try to work on either ways and figure out which one is more suitable. Thanks again!

@Eddy114514
Copy link
Collaborator Author

@ltratt
After discussed and get help from @nanjekyejoannah about bstate yesterday, I choose to return a special float-like object (subclass of float object) from random.random. And since random.random return float object, I move check from len of string object to __str__ of float-like object (I called it pg_float).

I avoid putting a new filed inside of float object since change size of struct might cause some chain reaction.

@ltratt
Copy link
Member

ltratt commented Oct 30, 2025

This looks interesting but dumb question: doesn't this still mean that everyone who does len(random.random()) gets the warning i.e. there's no way to silence it?

@Eddy114514
Copy link
Collaborator Author

This looks interesting but dumb question: doesn't this still mean that everyone who does len(random.random()) gets the warning i.e. there's no way to silence it?

@ltratt it seems that len(random.random()) won't run because float objects doesn't have such method. As picture shown below Screenshot_20251030_042311_Chrome.jpg

@ltratt
Copy link
Member

ltratt commented Oct 30, 2025

Sorry, I meant len(str(random.random())).

@Eddy114514
Copy link
Collaborator Author

@ltratt I think since we are intended to warn user about the length change of str(random.random()), so user should get warning by directly calling len() on it. And when user change their code, for code like if (len(str(random.random())) == 14 (length of random.random() in python2), their could be 2 way:

  • if(len(str(random.random())[:14]) == 14
  • if(len(str(random.random())) == 18

We could avoid warning in first case that we also store the extra information about whether generated from random in string-object and check in len() method that whether it is from random and with length of 18. But for second case, unless we also store the information inside of integer return by len(), we might can't avoid false positive.

We could maybe also maintain a global dictionary with key be location of warning (code line number, etc) and value be a bool of whether user change their code when user run pygrate3 on their code that we assume user's change is correct and mark bool at that location to be true and avoid throw warning to user in next check. But if we assume user's change is correct then 1 run of pygrate3 is enough such that we don't even need to worry about silence problem.

@nanjekyejoannah
Copy link
Collaborator

@Eddy114514 lets first find a justification for duplicate C code on floats to avoid duplication.

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