search

radiocosmology / caput

Cluster Astronomical Python Utilities
GNU General Public License v2.0
12 stars 21 forks source link

fix(tod.concatenate): Convert index map to unicode. #201

Closed tristpinsm closed 2 years ago

jrs65 commented 2 years ago

@tristpinsm do we really need to deal with the case where the concatenation axis has a string type? I struggle to see when a time like axis that is fine for concatenation would have a string type.

jrs65 commented 2 years ago

That would obviously simplify this PR as you could remove two of the chunks.

tristpinsm commented 2 years ago

@tristpinsm do we really need to deal with the case where the concatenation axis has a string type? I struggle to see when a time like axis that is fine for concatenation would have a string type.

Yeah, I dunno. If you did pass it a dataset that fits that description with convert_dataset_strings=True wouldn't you expect it to do the conversion? I also can't think of an example where that happens but I don't see any reason why it's impossible.

In any case I'm fine with removing that part, just let me know.

jrs65 commented 2 years ago

I think it might rely on the axis being sanely sortable.

On Wed, Jun 29, 2022 at 7:38 PM tristpinsm @.***> wrote:

@tristpinsm https://github.com/tristpinsm do we really need to deal with the case where the concatenation axis has a string type? I struggle to see when a time like axis that is fine for concatenation would have a string type.

Yeah, I dunno. If you did pass it a dataset that fits that description with convert_dataset_strings=True wouldn't you expect it to do the conversion? I also can't think of an example where that happens but I don't see any reason why it's impossible.

In any case I'm fine with removing that part, just let me know.

— Reply to this email directly, view it on GitHub https://github.com/radiocosmology/caput/pull/201#issuecomment-1170284147, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAFJXJQTUAG76DFYXZSNSHDVRSCYDANCNFSM5ZKN77AQ . You are receiving this because your review was requested.Message ID: @.***>