py3: hash for padic extension rings

[chapoton]

part of #24551

[chapoton]

New commits:

​5193635

py3: hash for p-adic extension rings

[caruso]

I'm not sure what hashes are used for but I would say that the precision type should be included in the hash, shouldn't it?

[chapoton]

This hash is parallel to the behaviour of __eq__ in the same class.

I you want to change the equality, maybe do that in another ticket ?

[chapoton]

some failing doctests..

[git]

Branch pushed to git repo; I updated commit sha1. New commits:

​8c1bde6

fix one detail in #26088

[chapoton]

This should fix the failing doctests.

[chapoton]

green bot, please review

[caruso]

I think that it's not enough to compare the string representation of the printer. For example:

sage: A.<pi> = Zp(2, print_mode='bars', print_sep='|').extension(x^3 - 2)
sage: B.<pi> = Zp(2, print_mode='bars', print_sep='#').extension(x^3 - 2)
sage: A == B
False
sage: hash(A) == hash(B)
True

Probably, one should implement a method __hash__ for the printer as well...

[git]

Branch pushed to git repo; I updated commit sha1. New commits:

​a7bff82

better hash #26088

[chapoton]

The hash is not necessarily taking all parameters into account. One could very well just forget about the printing mode.

Nevertheless, here is another proposal.

[caruso]

Replying to chapoton:

The hash is not necessarily taking all parameters into account. One could very well just forget about the printing mode.

What's exactly the purpose and the requirements on the hash? Do we need to ensure that A == B implies hash(A) == hash(B)? Or the other way around? Something else? Nothing?

[chapoton]

The only requirement is that "A == B" implies "hash(A) == hash(B)".

One could even use the constant hash, that would be perfectly valid, but useless. So in general there is a balance or a choice : complex hash that tells many things apart, or simple hash that is not so good.

I think in sage, the hash for parents are not so important. They are required for python3 nevertheless.

​https://docs.python.org/2/library/functions.html#hash

​https://docs.python.org/2/glossary.html#term-hashable

[chapoton]

Is this ok for you, Xavier ? Patchbot is green.

[caruso]

Yes, it looks good to me.

I would be happy to give a positive review but I don't know so much about p-adic printers and I would be more comfortable if David confirms that your changes won't break something elsewhere. David, could you do this, please?

[saraedum]

A printer's __eq__ is quite complex so I think that hashing the full dict is too much, i.e., it'll give different hashes to printers that are considered equal. Strangely, the printer is not hashable (seems like a bug to me) so I'd not include it into the hash of the ring at all. If it were hashable, we should just include the printer's hash value.

So, I'd drop hash_printer and add a comment that printer's are strangely not hashable. Does that sound Ok to you?

[git]

Branch pushed to git repo; I updated commit sha1. New commits:

​4e06874

do not hash the _printer in #26088

[chapoton]

done

[chapoton]

Everything seems to be ok. Positive review, somebody ?

[chapoton]

review, please