Refpts handling not working as intended? about at HOT 2 CLOSED

Hello Tobyn. Glad to see you back !
First one word about your 2nd issue: docstrings. You are right to ask for a single message in all functions. For me, the best one (the most recently written !) is the one in linopt and ohmi_envelope. It can be improved as:

        refpts          elements at which data is returned. It can be
                        1) an integer in the range [-len(ring), len(ring)-1] selecting the
                           element according to python indexing rules. As a special case,
                           len(ring) is allowed and refers to the end of the last element,
                        2) an ordered list of such integers,
                        3) a numpy array of booleans of maximum length len(ring)+1, where
                           selected elements are true.
                        Defaults to None
...
        All values are given at the entrance of each element specified in refpts

and copied (or referenced to) everywhere. The additional note in lattice_pass only points out evidences, it can be removed though I do not see how it contradicts with the other descriptions.

About your table of tests: only the 1st line shows an error. In all other cases; the behaviour is correct and the error message could be improved (something like 'unordered list or element out of range')

Conclusion 2: the maximum length of refpts is indeed n_elements+1 for both integer and boolean inputs. It can be shorter in both cases, even empty, and missing booleans are considered False. A longer array implies there is a duplicate, and such an error is already correctly reported, so there is no need for an additional check (assuming the message is improved). Except in the case of the 1st line of your table: you are right, the check is skipped. Beware: contrary to what you wrote, refpts=n_elements (not n_elements+1) refers to the same as refpts=0.

Conclusion 3: strictly speaking, the useful range is [-n_elems, n_elems]. However , values outside that range are properly handled assuming a closed ring: k*n_elems always refers to the 1st element for any k, and so on. Any additional test must be almost transparent since uint32_refpts can be called repeatedly many times in functions calling another one. That's probably why I assumed (wrongly) that if the type was already uint32, the check had already been done previously…

Conclusion 4: you are welcome of course to factorise any piece of code commonly used ! In this particular case, I suggest that the name uint32_refpts still groups both parsing and conversion: it is used for argument checking at the beginning of almost any AT function where both are needed. And a pure validity check is not useful in bool_refpts since the boolean mask already ensures range, ordering and unicity of references.

from at.

@lfarv Thank you. My apologies, my confusion about what was equivalent to refpts=0 led me to believe this was a larger issue.

On the topic of docstrings, I think your new proposed explanation is much more understandable; the only thing I would ask is that we specifically mention that duplicates are not allowed, perhaps: 2) an ordered list of such integers without duplicates,

Though it is unusual I do not have any problem with allowing indexes outside the specified range to be used in a wrap-around style, if it is intended behaviour. That said considering we now clearly state the range of valid values for refpts in the new docstring, it would be expected behaviour as far as the user is concerned for any value outside that range to raise an error. So if we are to keep allowing this, I would recommend either raising a warning or having a specific note about this use case in the docstring.

As for the error message, I am happy so long as it specifically mentions all three "throw" cases:

1. unordered list,
2. duplicates in list,
3. value outside of range resulting in duplicate upon conversion to valid refpoint

If refpts of data type uint32 are checked properly too, then fully considering the above I do not think uint32_refpts needs rewriting.

from at.