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

Feature/fs selection #56

Merged
merged 5 commits into from
Oct 23, 2024
Merged

Feature/fs selection #56

merged 5 commits into from
Oct 23, 2024

Conversation

TheSinnerAR
Copy link
Collaborator

Se agrega algoritmo de BKMEANS y se fixea la configuracion para poder consumir las APIs publicas de BIOAPI y MODULECTOR

…ingAlgorithm in models.py)

- [src/feature_selection/models.py]
- [src/feature_selection/fs_models.py]
- [src/feature_selection/views.py]
- [src/frontend/static/frontend/src/components/biomarkers/labels/ClusteringAlgotithmLable.tsx]
- [src/frontend/static/frontend/src/components/biomarkers/types.ts]
- [src/frontend/static/frontend/src/components/biomarkers/utils.ts]
if modulector_settings['protocol'] == 'http' and modulector_settings['port'] == 80:
self.url_modulector_prefix = f"{modulector_settings['protocol']}://{modulector_settings['host']}"
elif modulector_settings['protocol'] == 'https' and modulector_settings['port'] == 443:
self.url_modulector_prefix = f"{modulector_settings['protocol']}://{modulector_settings['host']}"
Copy link
Member

@Genarito Genarito Oct 17, 2024

Choose a reason for hiding this comment

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

Tanto esta línea como la del caso de "http" son lo mismo, yo lo que haría sería:

  1. Armar la URL con el protocolo y sin el puerto
  2. Lo que veo que estás haciendo es ignorar el puerto en caso de que sea el que va por defecto, así que pondría el if y elif en un solo if con un simple or preguntando si el puerto debería ser incluído a la URL o no.
  3. Entonces lo que haría adentro de este if sería agregar el puerto
  4. Documentar qué es lo que se está haciendo con un comentario
  5. Poner en una función dentro de la clase para que no sea código duplicado con lo de BioAPI

Te quedaría algo como esto:

modulector_settings = settings.MODULECTOR_SETTINGS
self.url_modulector_prefix = f"{modulector_settings['protocol']}://{modulector_settings['host']}"

# Removes the port from the URL if it is the default port for the protocol
if ((modulector_settings['protocol'] == 'http' and modulector_settings['port'] != 80) or 
    (modulector_settings['protocol'] == 'https' and modulector_settings['port'] != 443)):
    self.url_modulector_prefix += f":{modulector_settings['port']}"

Te dejo el punto 5 para corregir esto y lo de BioAPI abajo!

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Se tomaron en cuenta todos los puntos mencionados, por lo que se creó una función con la lógica necesaria para no repetir funcionalidad y se agregaron comentarios

modulector_settings = settings.MODULECTOR_SETTINGS
self.url_modulector_prefix = self.__build_url(modulector_settings)

bioapi_settings = settings.BIOAPI_SETTINGS
self.url_bioapi_prefix = self.__build_url(bioapi_settings)
protocol = settings['protocol']
host = settings['host']
port = settings['port']

if (protocol == 'http' and port == 80) or (protocol == 'https' and port == 443):
    return f"{protocol}://{host}"
else:
    return f"{protocol}://{host}:{port}"

@@ -33,6 +33,7 @@ class ClusteringAlgorithm(models.IntegerChoices):
"""Clustering algorithm."""
K_MEANS = 1
SPECTRAL = 2 # TODO: implement in backend
BK_MEANS = 3
Copy link
Member

Choose a reason for hiding this comment

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

Siempre que se cambia un campo que va hacia un modelo se tiene que correr el comando python3 manage.py makemigrations y luego python3 manage.py migrate así la DB se entera.
Ese archivo .py que es la migración también pusheala a este PR por fa!

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Se agregaron las migraciones faltantes 👨‍💻

@TheSinnerAR
Copy link
Collaborator Author

  • Se agregaron las correcciones necesarias para el correcto funcionamiento del servicio externo para el consumo de APIs
  • El algoritmo de BKMeans está implementado

Se configura para dejar como default el consumo de Modulector y Bioapi de manera online
@TheSinnerAR
Copy link
Collaborator Author

Se configura para que de manera default se consuma los servicios online de Modulector y BioApi

@Genarito Genarito merged commit adb2585 into v5.0.0 Oct 23, 2024
1 check passed
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.

2 participants