Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

passing a single Session to stack leads to unexpected result #1057

Closed
gdementen opened this issue Mar 3, 2023 · 0 comments
Closed

passing a single Session to stack leads to unexpected result #1057

gdementen opened this issue Mar 3, 2023 · 0 comments

Comments

@gdementen
Copy link
Contributor

>>> mul2 = ndtest(3) * 2
>>> mul2
a  a0  a1  a2
    0   2   4
>>> mul10 = ndtest(3) * 10
>>> mul10
a  a0  a1  a2
    0  10  20
>>> sess = Session(mul2=mul2, mul10=mul10)
>>> stack(sess, 'array=mul10,mul2')
a\array  mul10  mul2
     a0      0     0
     a1      2    10
     a2      4    20

The problem is that sess is just seen as an iterable in this case, so is taken as a list, discarding its labels.

I knew it was not implemented (see discussion in #180) but I did not realize that the fact that it seemed to work can lead to silent data garbage when users use this and do not realize the labels do not correspond to the correct data anymore.

I think that fixing this is simply a matter of adding the following lines to stack:

    elif isinstance(elements, Session):
        items = elements.items()

We might want to have a default axis name in that case:

        if axes is None:
            axes = 'array'

but I am unsure about this, and if we go down that path, we should have "sensible" default labels in other places too.

@gdementen gdementen added this to the 0.34.1 milestone Mar 3, 2023
@gdementen gdementen self-assigned this Jun 26, 2023
gdementen added a commit to gdementen/larray that referenced this issue Sep 12, 2023
…ect#1057)

I consider this as a bug fix because stack(Session) *seemed* to work but did not keep labels
(because the Session was considered as a Sequence), which is surprising and caused issue in production code
gdementen added a commit to gdementen/larray that referenced this issue Sep 13, 2023
…ect#1057)

I consider this as a bug fix because stack(Session) *seemed* to work but did not keep labels
(because the Session was considered as a Sequence), which is surprising and caused issue in production code
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

1 participant