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

Stack.py now preserves attrs #210

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

Conversation

griffbad
Copy link

took me a while to figure out why my attrs data (initially found from using extra_data in SpecifiedLocation) was not preserved at end of pipeline. This was it.

Pre-Merge Checklist:

  • if this PR adds a feature: request merge into the latest development branch (vX.Y-dev)
  • [X ] if this PR fixes a bug: request merge into the latest patch branch (patch-X.Y.Z)
  • PR branch name is short and describes the feature/bug addressed in this PR
  • commit messages are consistent
  • changes reviewed by another contributor
  • tests cover changes
  • all tests pass

took me a while to figure out why my attrs data (initially found from using extra_data in SpecifiedLocation) was not preserved at end of pipeline. This was it.
@pattonw
Copy link
Collaborator

pattonw commented Jun 11, 2024

Thanks for bringing this to my attention!

The whole extra_data field needs to be reworked. We are moving away from using .attrs and instead just providing extra data in "non-spatial" arrays that are still managed by requests. This way there's no unexplained data being passed around.

In the mean time until .attrs is completely removed, I think this seems like a reasonable patch for the current behavior. I would only make sure that the value stored in .attrs remains a dictionary instead of being turned into a list as it is done here. Something like just assuming the set of keys in each batch do not overlap and then turning them into a larger dict with a union across the keys.

Copy link

github-actions bot commented Jul 3, 2024

This PR will be closed in one week due to inactivity. Please update the PR if necessary.

6 similar comments
Copy link

This PR will be closed in one week due to inactivity. Please update the PR if necessary.

Copy link

This PR will be closed in one week due to inactivity. Please update the PR if necessary.

Copy link

github-actions bot commented Sep 5, 2024

This PR will be closed in one week due to inactivity. Please update the PR if necessary.

Copy link

This PR will be closed in one week due to inactivity. Please update the PR if necessary.

Copy link

This PR will be closed in one week due to inactivity. Please update the PR if necessary.

Copy link

github-actions bot commented Nov 8, 2024

This PR will be closed in one week due to inactivity. Please update the PR if necessary.

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 this pull request may close these issues.

2 participants