Poset: change rank dict to rank array

[jmantysalo]

On hasse_diagram.py there is now _rank_dict whose keys are just numbers 0,...,n. It can be converted to array, and it will be slightly faster then.

[git]

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

​08727b2

trac #17191: Poset: change rank dict to rank array

[jmantysalo]

About

return lambda x:self._rank[x] # the rank function is just the getitem of the dict

The comment string should be changed, "dict" -> "list". Also why bother with lambda function, because lists also has a __getitem__?

Line

return [rank_fcn[i] for i in range(self.order())]

seems strange --- isn't that just same as return rank_fcn or return copy(rank_fcn)? Or maybe I didn't understand this.

[ncohen]

Yooo !

return lambda x:self._rank[x] # the rank function is just the getitem of the dict

The comment string should be changed, "dict" -> "list". Also why bother with lambda function, because lists also has a __getitem__?

You are right.

return [rank_fcn[i] for i in range(self.order())]

seems strange --- isn't that just same as return rank_fcn or return copy(rank_fcn)? Or maybe I didn't understand this.

Because rank_fcn is a dictionary, not a list, and because it should be a dictionary in this function (because of lines like 'if x in rank_fcn').

Nathann

[git]

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

​45fc474

trac #17191: Reviewer's comments

[jmantysalo]

Replying to ncohen:

return [rank_fcn[i] for i in range(self.order())]

seems strange --- isn't that just same as return rank_fcn or return copy(rank_fcn)? Or maybe I didn't understand this.

Because rank_fcn is a dictionary, not a list, and because it should be a dictionary in this function (because of lines like 'if x in rank_fcn').

Ah, true. This could also start with rank_fcn=[None]*self.order() instead of rank_fcn={}, but the current code seems to be cleaner.

I think this is ready for production, so I also changed milestone with status.

[ncohen]

Yo !

Ah, true. This could also start with rank_fcn=[None]*self.order() instead of rank_fcn={}, but the current code seems to be cleaner.

Right again. And I could have turned the 'if x in rank_fcn' into 'if rank_fcn[x] is Null'... Stupid me.

I think this is ready for production, so I also changed milestone with status.

I hope that you do not mind: I am going to add a commit in a second to do that properly with a list in the function too. Not very important, but worth doing anyway.

Nathann

[git]

Branch pushed to git repo; I updated commit sha1 and set ticket back to needs_review. New commits:

​cc8fd43

trac #17191: Doing the job properly

[jmantysalo]

Works.

[ncohen]

Thanks !

Nathann