dominicprice / endplay

A suite of tools for generation and analysis of bridge deals. Read the documentation at https://endplay.readthedocs.io
MIT License
21 stars 5 forks source link

Some issue with Rank #15

Closed LyonsDo closed 1 year ago

LyonsDo commented 2 years ago

There is really a lot of very useful material in this project - thanks.

This is a very minor (non)issue - PBN2.1 says "The import format is rather flexible and is used to describe data that may have been prepared by hand." That I suppose covers the deal format I was reading in [Deal "S:SA7.H864.DQJT73.CAKQ SQ2.H952.D62.CJT9742 SKT853.HT3.D954.C653 SJ964.HAKQJ7.DAK8.C8"] where the suits are redundant. Not surprisingly

@staticmethod def find(value: str) -> 'Rank': return Rank[f"R{value.upper()}"]

returns "RS"

Just FYI.

dominicprice commented 2 years ago

Hiya, I've not seen seen PBN written like that before, and it's not mentioned in 3.4.11 as a possibility but no doubt if you've come across it then it will exist in other places in the wild... In any case should be an easy fix and there are a couple more things in the PBN parser which are on my todo list anyway so its a good excuse for me to go and do some tinkering.

Dom

LyonsDo commented 2 years ago

Thanks for the response Dom. Everything exists in the wild but I had an easy fix by globally replacing the suits. I've just come across a real issue where Spade voids don't get parsed correctly. C, D, H are OK in the following: [Deal "S:AKQ6.T753.AJ7.Q2 JT52.Q9864.KT43.. 9873.J2.Q8.KJ843 4.AK.9652.AT9765"] [Deal "S:K643.Q8.KQJ.A872 AQJ8.K654.A752.K T.JT7.T98643.Q63 9752.A932..JT954"] [Deal "S:Q62.AK7.K97.QJ84 83.QJ942.642.K63 54.T8653.AJT53.2 AKJT97..Q8.AT975"] [Deal "S:A2.KJ7.QJ643.A85 Q43.AT83.AK952.T KJT98765.2.8.J43 ..Q9654.T7.KQ9762"] but S throws an index error. And later in the file I noticed a case where parsing might possibly be even trickier. [Deal "S:AK65.986.QT2.AQJ QT72.J2.764.T865 J9843.KT543.AJ9.. ..AQ7.K853.K97432"]

dominicprice commented 2 years ago

I'll investigate whats going on there!

dominicprice commented 2 years ago

Hiya, In this case it looks like the problem is hands containing more than three periods, so e.g. in the last one which throws the error the last hand is ..AQ7.K853.K97432, so it assigns a void to both spades and hearts, which it splits into five different suit holdings the last of which gets assigned to the non-existent fifth suit causing the index error. The hand just before, J9843.KT543.AJ9.., also contains four periods but as in this instance the final one is a void it never actually attempts to assign anything for this suit so it doesn't complain.

I can add code to catch this, but I think the correct behaviour should be to emit an error message that the hand contains more than four suits rather than silently try and 'fix' the pbn as it is too ambiguous.

LyonsDo commented 2 years ago

Thanks Dom. That's it OK and I agree with your error message.

In this case, the wild is me :-( I wrote a Lua program for Piotr Beling's bdeal system. Should be straightforward to fix it.

dominicprice commented 1 year ago

Closing, this was fixed a while ago but I forgot to follow up on this thread.