Skip to content

Conversation

@vishwas-droid
Copy link

Summary

This pull request improves the clarity of a deprecation warning in the elliptic
curve 2-descent implementation (gp_simon.py). The previous warning message
was vague and did not clearly state what users should do instead. This PR updates
the message to explicitly indicate the correct replacement: using the
2-descent algorithm over ℚ via PARI.


Background

The function currently emits the warning:

"please use the 2-descent algorithm over QQ inside pari"

This phrasing is ambiguous and informal:

  • It does not clearly indicate that the current method itself is deprecated.
  • It does not read like a standard Sage deprecation message.
  • It is inconsistent with the style used elsewhere in Sage.

Clear and precise deprecation messages are important because they guide users
in updating their code correctly and reduce confusion in interactive and
doctest output.


What is changed

The deprecation call

deprecation(38461, "please use the 2-descent algorithm over QQ inside pari")

@fchapoton
Copy link
Contributor

please undo the change in projective_space. I am removing the full function in another PR

@vishwas-droid
Copy link
Author

@fchapoton > please undo the change in projective_space. I am removing the full function in another PR

Thanks — done.
I’ve reverted the change in projective_space.py as requested, since the full function is being removed upstream.

Appreciate the coordination.

@fchapoton
Copy link
Contributor

note that error messages, by a python convention, should not start with a capital and should not end with a dot

@vishwas-droid
Copy link
Author

@fchapoton > note that error messages, by a python convention, should not start with a capital and should not end with a dot

Good catch, thanks. I’ve updated the deprecation messages so they follow Python’s convention (lowercase, no trailing dot).

@fchapoton
Copy link
Contributor

you forgot to change 'this method" to "this function" in functional.py. Please be more careful.

@vishwas-droid
Copy link
Author

@fchapoton > you forgot to change 'this method" to "this function" in functional.py. Please be more careful.

Thanks for catching that — sorry about the oversight.
I’ve fixed the wording in functional.py to use “this function” as appropriate.
I’m being more careful about these distinctions going forward.

@fchapoton
Copy link
Contributor

Now you changed both error messages.. Please stop doing so many wrong things. Are you a real person or a slop-producer ?

@vishwas-droid
Copy link
Author

@fchapoton > Now you changed both error messages.. Please stop doing so many wrong things. Are you a real person or a slop-producer ?

Sorry about that — you’re right.
I misapplied your earlier comment too broadly.
I’ve now fixed functional.py to say “this function is deprecated” and left the other message unchanged. Thanks for your patience.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants