-
Notifications
You must be signed in to change notification settings - Fork 1
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
Cli #15
base: main
Are you sure you want to change the base?
Conversation
# Conflicts: # bayesian_cnn_prometheus/preprocessing/data_splitter.py
config = get_mask_analyser_config() if len(sys.argv) < 4 else MaskAnalysisConfig('', '', '') | ||
|
||
model_name = get_arg(1, config.model_name) | ||
lesion_masks_path = get_arg(2, config.lesion_masks_path) | ||
variance_masks_path = get_arg(3, config.variance_masks_path) | ||
|
||
analise(model_name, lesion_masks_path, variance_masks_path) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Z ciekawości: czemu nie użyłeś tutaj click'a?
default=Paths.RESULTS_PATH, | ||
type=click.STRING, | ||
help='Path to the variance masks') | ||
@click.option('--model-name') |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Może dać też krótką flagę -m
?
A na maskach dać -l
(od lesions)`
@@ -24,18 +24,22 @@ class Paths: | |||
MASKS_DIR = 'LESIONS' | |||
MASKS_PATH = DATA_DIR / MASKS_DIR | |||
MASK_FILE_PATTERN = 'MASK_{}.{}' | |||
MASK_FILE_PATTERN_WITH_DIR = Path(MASKS_DIR) / MASK_FILE_PATTERN |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Czemu chcesz zamienić MASK_FILE_PATTERN_PATH
na MASK_FILE_PATTERN_WITH_DIR
?
Mi się wydaje, że takie mnożenie zmiennych jest trochę niewygodne, bo potem będzie trudno się zoorientować co do czego się odnosi.
assert_fields_have_values(app_config.get('mask_analysis', {}), [ | ||
field.name for field in fields(MaskAnalysisConfig)]) | ||
config = MaskAnalysisConfig(**app_config['mask_analysis']) | ||
def analise(model_name: str, variance_masks_path: str, lesion_masks_path: str): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Literówka, analyse/analyze
mask_analyzer = MasksAnalyzer( | ||
config.model_name, config.lesion_masks_path, config.variance_masks_path) | ||
|
||
def analise_from_config(config: MaskAnalysisConfig): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Jak wyżej, literówka
mask_analyzer = MasksAnalyzer(config.model_name, config.lesion_masks_path, config.variance_masks_path) | ||
mask_analyzer.perform_analysis(save_to_json=True) | ||
|
||
|
||
def get_mask_analyser_config(): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Moim zdaniem powinniśmy ujednolicić tutaj konwencję - albo MaskAnalyzer
i get_mask_analyzer_config
, albo analogicznie pisownia z 's'
Jako że nigdy nic nie robiłem na prometeuszu prosiłbym o sprawdzenie, albo pokazanie mi jak to mogę zrobić :)
Kod zrobiłem tak, aby w dalszym ciągu była możliwość uruchomienia mainów osobno, a nie z konsoli.