[bugifx] Make sure region edge units are in code_length.

[brittonsmith]

After PR #2391 was merged, I encountered a few cases of all_data selection returning empty arrays. It seems in some cases the region edges are not in code units.

[matthewturk]

This looks right to me, but any chance you can say more about which situations it might happen?

[brittonsmith]

Sure, I encountered this making lightray data sets with Trident. The underlying data format is ytdata. I think data set is saved with domain_left_edge and domain_right_edge defined in unitary units. When I was debugging this, that's what region.left_edge and region.right_edge were in.

I'm happy to share the lightray file if you want to play around, but it may be more trouble than it's worth to add a test that uses it.

[matthewturk]

This error is odd -- it looks like a transient AppVeyor issue, maybe? It's nothing related to this PR, so I think we should accept this and address the AppVeyor issue (which seems to be showing up elsewhere) in another PR.

[brittonsmith]

I'll just add this for anyone interested. The problem can be demonstrated by loading up a particle dataset and doing the following:

ds = yt.load(...)
ds.domain_right_edge = ds.domain_right_edge.to('unitary')
ds.domain_left_edge = ds.domain_left_edge.to('unitary')

Not that I'm condoning that operation, but it demonstrates what happens when the default domain units are not code_length, as in the case I encountered.

[matthewturk]

@brittonsmith I am not sure I understand how this is possible -- aren't domain_left_edge and domain_right_edge instances of MutableAttribute objects, which shouldn't allow them to be reset?

Also, would this problem be solved if all of our "index"-related arrays were immutable?

[brittonsmith]

You can still do

ds.domain_right_edge = 'reginaldbarclay'

and it will stick.

The MutableAttribute only prevents the units from changing permanently with convert_to_units.

The primary issue of this bugfix is that there is a an assumption within the selection routines that domain bounds are always in code_length, but there are instances where that isn't true.

[brittonsmith]

Any objections to merging this?

[matthewturk]

Nope.

…

On Thu, Jan 23, 2020 at 10:14 AM Britton Smith ***@***.***> wrote: Any objections to merging this? — You are receiving this because you commented. Reply to this email directly, view it on GitHub <#2399?email_source=notifications&email_token=AAAVXOZFR6HKBEAKMGA7RGDQ7G643A5CNFSM4J3ZCON2YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOEJX4ZNI#issuecomment-577752245>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAAVXO453QDGM7POLLD6DJTQ7G643ANCNFSM4J3ZCONQ> .