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

fix: Remove circular imports and add missing import #105

Merged
merged 14 commits into from
Feb 15, 2024
Merged

Conversation

adjavon
Copy link
Contributor

@adjavon adjavon commented Feb 15, 2024

Note that this still does not pass tests.

Copy link
Member

@rhoadesScholar rhoadesScholar left a comment

Choose a reason for hiding this comment

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

See comments por favor

@@ -2,7 +2,6 @@
from dacapo.blockwise.scheduler import run_blockwise
from dacapo.compute_context import ComputeContext, LocalTorch
from dacapo.experiments.datasplits.datasets.arrays.zarr_array import ZarrArray
from dacapo.store.array_store import LocalArrayIdentifier
Copy link
Member

Choose a reason for hiding this comment

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

Needs to still be included

rhoadesScholar and others added 3 commits February 15, 2024 11:03
There appear to be some python formatting errors in
e42012f. This pull request
uses the [psf/black](https://github.com/psf/black) formatter to fix
these issues.
One of the files was using `LocalArrayIdentifier`, so in order to remove
the circular import entirely we had to empty `store.__init__`.
This means that from this point on, all of the `create_*_store` calls
need to refer directly to `dacapo.store.create_store`
dacapo/train.py Outdated
create_weights_store,
)
from dacapo.experiments import Run
from dacapo.compute_context import LocalTorch, ComputeContext
from .validate import validate_run
Copy link
Member

Choose a reason for hiding this comment

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

can you make this absolute?

@rhoadesScholar
Copy link
Member

Make it pass tests please! :)

@rhoadesScholar rhoadesScholar changed the base branch from main to dev/main February 15, 2024 17:03
@adjavon adjavon marked this pull request as draft February 15, 2024 17:09
@rhoadesScholar rhoadesScholar marked this pull request as ready for review February 15, 2024 18:03
@rhoadesScholar rhoadesScholar merged commit a14cf2f into dev/main Feb 15, 2024
0 of 6 checks passed
@rhoadesScholar rhoadesScholar deleted the hotfix branch February 15, 2024 18:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: Release
Development

Successfully merging this pull request may close these issues.

2 participants