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

[9/5] ENH: replace hidden imports with public ones #405

Merged
merged 2 commits into from
Aug 15, 2018

Conversation

ambrosejcarr
Copy link
Member

This PR:

  • Adds SpotAttributes as a public object in starfish.spots
  • Fixes all private imports in the non-test code.

@ambrosejcarr ambrosejcarr requested a review from ttung August 14, 2018 00:23
@ambrosejcarr ambrosejcarr changed the title ENH: replace hidden imports with public ones [9/5] ENH: replace hidden imports with public ones Aug 14, 2018
@codecov-io
Copy link

codecov-io commented Aug 14, 2018

Codecov Report

Merging #405 into ajc-remove-private-imports-and-validate-notebooks will decrease coverage by 0.13%.
The diff coverage is 100%.

Impacted file tree graph

@@                                  Coverage Diff                                  @@
##           ajc-remove-private-imports-and-validate-notebooks     #405      +/-   ##
=====================================================================================
- Coverage                                              85.71%   85.58%   -0.14%     
=====================================================================================
  Files                                                     68       68              
  Lines                                                   2668     2713      +45     
=====================================================================================
+ Hits                                                    2287     2322      +35     
- Misses                                                   381      391      +10
Impacted Files Coverage Δ
starfish/stack.py 72.34% <100%> (ø) ⬆️
starfish/spots/_detector/local_max_peak_finder.py 93.18% <100%> (ø) ⬆️
starfish/spots/_detector/pixel_spot_detector.py 56.25% <100%> (ø) ⬆️
starfish/spots/__init__.py 100% <100%> (ø) ⬆️
starfish/spots/_detector/_base.py 80.95% <100%> (ø) ⬆️
starfish/spots/_detector/detect.py 98.46% <100%> (ø) ⬆️
starfish/spots/_detector/gaussian.py 100% <100%> (ø) ⬆️
starfish/starfish.py 83.01% <100%> (ø) ⬆️
starfish/image/_segmentation/watershed.py 73.61% <0%> (+1.35%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 13d4636...de0b612. Read the comment docs.

Copy link
Collaborator

@ttung ttung left a comment

Choose a reason for hiding this comment

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

see the comments.

from starfish.intensity_table import IntensityTable
from starfish.munge import dataframe_to_multiindex
from starfish.spots._spot_attributes import SpotAttributes
from starfish.stack import ImageStack
Copy link
Collaborator

Choose a reason for hiding this comment

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

sort

@@ -2,13 +2,13 @@

import numpy as np
import xarray as xr
from starfish.intensity_table import IntensityTable
Copy link
Collaborator

Choose a reason for hiding this comment

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

these are first party.

@ambrosejcarr ambrosejcarr force-pushed the ajc-remove-private-imports-and-validate-notebooks branch from 9eca5d3 to d631be4 Compare August 14, 2018 20:02
@ambrosejcarr ambrosejcarr force-pushed the ajc-reduce-private-imports branch from 0131e41 to 7ac70aa Compare August 14, 2018 20:06
@ambrosejcarr ambrosejcarr force-pushed the ajc-reduce-private-imports branch from 7ac70aa to 83c2804 Compare August 15, 2018 15:53
@ambrosejcarr ambrosejcarr force-pushed the ajc-remove-private-imports-and-validate-notebooks branch from 72fca8b to 52525d1 Compare August 15, 2018 15:59
@ambrosejcarr ambrosejcarr force-pushed the ajc-reduce-private-imports branch from 83c2804 to 1de340d Compare August 15, 2018 16:01
@@ -1,3 +1,4 @@
from ._spot_attributes import SpotAttributes
Copy link
Collaborator

Choose a reason for hiding this comment

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

sort it pls.

from starfish.spots._target_assignment import TargetAssignment
from starfish.image._registration import Registration
from starfish.image._segmentation import Segmentation
from starfish.spots import Decoder
Copy link
Collaborator

Choose a reason for hiding this comment

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

sort pls.

@ttung
Copy link
Collaborator

ttung commented Aug 15, 2018

wait i already approved. lol.

@ttung
Copy link
Collaborator

ttung commented Aug 15, 2018

Tests are failing though.

@ambrosejcarr ambrosejcarr force-pushed the ajc-remove-private-imports-and-validate-notebooks branch 3 times, most recently from a2ee3e9 to 13d4636 Compare August 15, 2018 21:12
@ambrosejcarr ambrosejcarr force-pushed the ajc-reduce-private-imports branch from 1de340d to de0b612 Compare August 15, 2018 21:14
@ambrosejcarr ambrosejcarr changed the base branch from ajc-remove-private-imports-and-validate-notebooks to master August 15, 2018 21:24
@ambrosejcarr ambrosejcarr merged commit 1e28935 into master Aug 15, 2018
@ambrosejcarr ambrosejcarr deleted the ajc-reduce-private-imports branch August 16, 2018 00:38
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.

3 participants