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

auto_inline not working as expected #599

Closed
nden opened this issue Nov 12, 2018 · 2 comments · Fixed by #802
Closed

auto_inline not working as expected #599

nden opened this issue Nov 12, 2018 · 2 comments · Fixed by #802

Comments

@nden
Copy link
Contributor

nden commented Nov 12, 2018

There must be a subtle bug in auto_inline - it seems the behavior depends on previously run commands. Is something setting a state?

f = AsdfFile({'data': np.arange(6)})
f.write_to('test.asdf') # correctly writes the data inline

f.write_to('test.asdf', auto_inline=3) # still writes it inline

# force it to write to a block
f.write_to('test.asdf', all_array_storage='internal') 

f.write_to('test.asdf', auto_inline=3) # writes to a block as expected

f.write_to('test.asdf', auto_inline=10) # writes it inline

f.write_to('test.asdf', auto_inline=3) # still writes it inline

@drdavella
Copy link
Contributor

Thanks for the report. I've noticed other places where write_to modifies the state of the AsdfFile object, which is not okay, in my opinion. I wish there was a better way to enforce this in Python (I'm thinking of const methods in C++, which can't update the associated object).

A related discussion came up in #597 regarding the use/implementation of inline_threshold. I think I'm going to have to rework that API and somehow combine it with auto_inline since they serve the same purpose. Since the inline_threshold feature is still in an unreleased version of ASDF, we have no concerns about making further API changes.

@drdavella
Copy link
Contributor

This is still a real issue, even after #610. Unfortunately, however, it's one that is unlikely to be resolved without a major redesign. I'm considering some major API changes for 3.0, which would hopefully address this kind of issue.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants