repr for SiteInst object could be improved

[mithro]

The current repr output of a SiteInst object looks like the following;

inst "null" "null",placed CLBLL_R_X17Y131 SLICE_X26Y131

Current confusing parts of this string are;

• The repr contains spaces and commas.
• The repr ends in a new line.

The current repr means when you do a print ("Sites:", site, si) you get something like;

('Sites:', SLICE_X26Y131, inst "null" "null",placed CLBLL_R_X17Y131 SLICE_X26Y131
)

Which looks like a tuple which has more then 3 elements...

I would suggest something like;

SiteInst(null, null, placed="CLBLL_R_X17Y131/SLICE_X26Y131")

[cn256]

Hi Tim,

I agree that the suggestion for having commas and not breaking the line feed after placed would be make it a bit easier to read. In some cases maybe the lines might get long. Maybe adding commas and moving the word "placed" to the next line with some indent might be an inbetween solution?

--Chris

On Tue, Feb 26, 2019 at 11:47 AM Tim Ansell notifications@github.com wrote:

The current repr output of a SiteInst object looks like the following;

inst "null" "null",placed CLBLL_R_X17Y131 SLICE_X26Y131

Current confusing parts of this string are;

• The repr contains spaces and commas.
• The repr ends in a new line.

The current repr means when you do a print ("Sites:", site, si) you get something like;

('Sites:', SLICE_X26Y131, inst "null" "null",placed CLBLL_R_X17Y131 SLICE_X26Y131 )

Which looks like a tuple which has more then 3 elements...

I would suggest something like;

SiteInst(null, null, placed="CLBLL_R_X17Y131/SLICE_X26Y131")

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/Xilinx/RapidWright/issues/23, or mute the thread https://github.com/notifications/unsubscribe-auth/ARSQT6cSPst5JQa4L1mQT_Pu98rZX857ks5vRY9ZgaJpZM4bS-WD .

[mithro]

I would say that people are use to repr strings getting long, so I wouldn't worry.

[clavin-xlnx]

As RapidWright objects are defined in Java, their "repr" string in Jython is really just the output of the class's toString(). However, @mithro has a good point that the current toString() is lacking (leftover from RapidSmith/XDL days). Most toString() calls simply return the name of the object or the Vivado-equivalent name. The SiteInst is a bit unique in that it takes multiple pieces of information to uniquely identify it. So I have mostly adopted the suggestion and will push an update with the following toString() pattern:

SiteInst(name="<name>", type="<site type enum>", site="<site name>")

for example:

SiteInst(name="null", type="null", site="null")
SiteInst(name="SLICE_X0Y0", type="SLICEL", site="SLICE_X0Y0")
SiteInst(name="Fred/SLICE_X0Y0", type="SLICEL", site="SLICE_X0Y0")