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

Rethinking external assets 2 #272

Merged
merged 24 commits into from
Oct 11, 2023

Conversation

evalott100
Copy link
Contributor

Closes #236

Putting on a new branch for cleanliness

@evalott100 evalott100 force-pushed the rethinking_external_assets_2 branch 7 times, most recently from 6b855c0 to 4649b1a Compare June 26, 2023 12:18
@evalott100 evalott100 force-pushed the rethinking_external_assets_2 branch from 3c0a582 to 2910dfe Compare June 30, 2023 10:43
…event_descriptor.py to use a Dtype as a type variable so we can import it from bluesky protocols
event_model/__init__.py Outdated Show resolved Hide resolved
… to only have one compose_datum in the ComposeStreamResourceBundle
@evalott100 evalott100 force-pushed the rethinking_external_assets_2 branch from 65cc064 to 78d95b4 Compare July 3, 2023 13:23
@evalott100 evalott100 force-pushed the rethinking_external_assets_2 branch 3 times, most recently from c565a96 to 010c410 Compare July 4, 2023 12:53
@evalott100 evalott100 force-pushed the rethinking_external_assets_2 branch from 010c410 to 6e8a865 Compare July 4, 2023 12:54
@evalott100 evalott100 force-pushed the rethinking_external_assets_2 branch from e011ca7 to 3c60781 Compare July 12, 2023 10:54
Copy link
Contributor

@coretl coretl left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just need that descriptor ref putting in

event_model/documents/stream_datum.py Outdated Show resolved Hide resolved
event_model/documents/stream_datum.py Show resolved Hide resolved
event_model/__init__.py Show resolved Hide resolved
@evalott100 evalott100 closed this Sep 28, 2023
@evalott100 evalott100 reopened this Sep 28, 2023
@danielballan
Copy link
Member

Oh, odd. I know we've retroactively applied black to some-but-not-all legacy bluesky repos. I can't remember for sure whether we did it here, but the single quotes in the codebase suggest we haven't.

@evalott100
Copy link
Contributor Author

evalott100 commented Sep 29, 2023

@danielballan

Found it!

01898f2

I'm unsure why these things are changing now though... Maybe a new black release, I think I see some highlights in the recent black releases which match up (I used 23.9.1 locally) 🤔

I'll run black on a different branch off of main and make a new PR.

Edit

Never mind, origin is already black compliant. This branch seems to have a very minor black change so I'll leave it here. Something must have been wrong locally in the last commit.

@danielballan
Copy link
Member

Ah, OK, that makes sense. I think we are good to merge then.

Copy link
Contributor

@coretl coretl left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Merge when this is fixed

event_model/documents/stream_resource.py Outdated Show resolved Hide resolved
Copy link
Contributor

@coretl coretl left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just realized we didn't finish the run_start: NotRequired[...] changes...

docs/user/explanations/data-model.rst Outdated Show resolved Hide resolved
event_model/documents/resource.py Outdated Show resolved Hide resolved
event_model/__init__.py Outdated Show resolved Hide resolved
event_model/__init__.py Outdated Show resolved Hide resolved
@coretl coretl merged commit e176a9c into bluesky:main Oct 11, 2023
11 checks passed
@evalott100 evalott100 deleted the rethinking_external_assets_2 branch October 11, 2023 10:22
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.

Rethinking of external assets
4 participants