Skip to content
This repository has been archived by the owner on May 31, 2023. It is now read-only.

Add Box3D, flake8 & cleanups #40

Merged
merged 6 commits into from
Feb 11, 2019
Merged

Add Box3D, flake8 & cleanups #40

merged 6 commits into from
Feb 11, 2019

Conversation

jstriebel
Copy link
Contributor

@jstriebel jstriebel commented Feb 8, 2019

This PR refactors the existing code to be much more readable.
It introduces

  • Box3D, which has math operators that simplify the code of the neuroglancer backend,
  • from_json can now also load Union tuples (useful for Optional),
  • flake8, that finds unused imports,
  • isort, which sorts the imports, and
  • some minor cleanups, e.g.
    • remove the precomputed:// prefix from the source links
    • do not use pre-releases anymore (except for black)

@jstriebel jstriebel self-assigned this Feb 8, 2019
@jstriebel jstriebel requested a review from fm3 February 8, 2019 17:41
Copy link
Member

@fm3 fm3 left a comment

Choose a reason for hiding this comment

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

Cool stuff! That will help a lot with readability :) Especially the Box3D.
Just to make sure, I assume removing tools/sample-datasets/neuroglancer.json was intentional?

@@ -4,21 +4,21 @@
"drosophila": {
"layers": {
"image": {
"source": "precomputed://gs://neuroglancer-fafb-data/fafb_v14/fafb_v14_clahe",
"source": "gs://neuroglancer-fafb-data/fafb_v14/fafb_v14_clahe",
Copy link
Member

Choose a reason for hiding this comment

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

👍

@jstriebel
Copy link
Contributor Author

Yes, removing tools/sample-datasets/neuroglancer.json was intentional, it's not necessary since we have data/datasets.json.

@jstriebel jstriebel merged commit 6d68507 into master Feb 11, 2019
@jstriebel jstriebel deleted the refactoring branch February 11, 2019 11:33
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants