Skip to content
This repository has been archived by the owner on Aug 13, 2020. It is now read-only.

WIP: OOP refactor #65

Merged
merged 33 commits into from
Aug 15, 2016
Merged

WIP: OOP refactor #65

merged 33 commits into from
Aug 15, 2016

Conversation

AlexS12
Copy link
Member

@AlexS12 AlexS12 commented Jun 19, 2016

Dado que esto estaba un poco muerto últimamente por la falta de tiempo generalizada y porque, creo, que en el estado actual del código es difícil hacer contribuciones, llevo un par de semanas introduciendo cambios.

Está lejos de estar terminado y quizá estoy siendo un poco desordenado con los commits, pero así está quedando el intento de hacer el código orientado a objetos.

Por un lado introduce la dificultad de trabajar con clases, pero simplificará mucho las interfaces y los casos de uso. Además será bastante fácil crear nuevos modelos (de aviones, atmósferas, ecuaciones...) usando los que ya están hechos como "plantillas". ¡Os avisaré cuando tenga algún ejempo funcionando! 😃

edit: y ahora que veo que se están intentando pasar los tests... PUES SEGURO QUE NO FUNCIONA NINGUNO! 😆 no hay que reescribirlos enteros, pero sí hace falta tocar un poquito. Estaría bien que si alguien tiene tiempo me echase una mano con eso.

@olrosales
Copy link
Member

@AlexS12 Alex igual que hiciste con GitHub podrias echarnos una mano con lo de los objetos?

@astrojuanlu
Copy link
Member

¡Genial @AlexS12! Me encargo yo de revisar esto, avisa cuando esté listo 🎉

@AlexS12
Copy link
Member Author

AlexS12 commented Jun 19, 2016

@olrosales para los que queráis aprender a trabajar con objetos:

  • Effective Computation in Physics (yo lo recomiendo para todo) da una visión básica en el capítulo 6 que debería ser suficiente.
  • Python 3 Object Oriented Programming es una referencia mucho más completa (pero también más ardua).

Cuando le hayáis echado un ojo al primero, principalmente, y esté terminado el código, hacemos una reunión y os explico el diseño y cómo se usa. y si vais teniendo dudas, lo vamos hablando.

Intentaré hacer una buena documentación del código antes de que se haga el merge. Estaría bien también una entrada en la wiki. Cuando estéis al tanto, espero que me podáis echar un cable.

@Juanlu001, estupendo os mantengo informados!

@olrosales
Copy link
Member

@AlexS12 Gracias Alex, lo que hay es que comentar bien el código. Todos en general

@AlexS12 AlexS12 mentioned this pull request Jun 21, 2016
@AlexS12
Copy link
Member Author

AlexS12 commented Jun 21, 2016

Antes de documentar todo, pulir unas cuantas cosas que faltan y arreglar y hacer tests, me gustaría saber qué os parece la interfaz orientada a objetos. 💭 Si lo organizaríais de otra manera, almacenaríais alguna cosa en otro lado, si se entiende algo... Le he dado unas cuantas vueltas desde que lo empecé y a esto he llegado. Si hace falta que os cuente como funciona o por qué he hecho alguna cosa así, nos juntamos en un hangouts próximamente. 😃

@astrojuanlu astrojuanlu added this to the v0.1 milestone Jul 7, 2016
@astrojuanlu
Copy link
Member

Aclarado el funcionamiento. Si te parece @AlexS12 termina de acomodar los tests y el resto del código para que todo encaje, y antes de fusionarlo le doy una revisión de estilo. Lo que hablamos de mejorar la API del trimador creo que se puede dejar para otro PR para no complicarnos mucho, si te parece bien.

@AlexS12
Copy link
Member Author

AlexS12 commented Jul 9, 2016

Vale, a ver si a lo largo de esta semana, rehago los test que fallan e incluyo alguno nuevo. A ver si antes de mergear la rama, tenemos entre @martosc y yo la documentación y el diagrama de cajas de #57. ¿Cómo lo ves de tiempos, Jesús?

@martosc
Copy link
Member

martosc commented Jul 9, 2016

Esta semana los diagramas de todo lo que tenemos lo veo justo. Te digo al final de este finde a ver cómo avanzo.

@AlexS12
Copy link
Member Author

AlexS12 commented Jul 9, 2016

Me refería más a la semana que viene en realidad, no de hoy al domingo, pero vamos, que no hay plazos

@astrojuanlu
Copy link
Member

@AlexS12 Los plazos están bien claros 😜

def calculate_forces_and_moments(self):
pass

@abstractmethod
Copy link
Member

Choose a reason for hiding this comment

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

¿Qué sentido tiene que esto sea un método abstracto, si ya tiene implementación? Me parece que este decorador sobra.

Copy link
Member Author

Choose a reason for hiding this comment

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

cierto!

@astrojuanlu
Copy link
Member

He hecho una revisión de estilo un poco superficial, por mi parte cuando @AlexS12 haga los cambios necesarios (o diga que no le da la gana hacerlos) integro esta pull request.

@AlexS12
Copy link
Member Author

AlexS12 commented Aug 14, 2016

Estoy haciendo pruebas con las otras condiciones de trimado y tengo algún problemilla todavía, vamos a esperar un poquito a hacer el merge. A ver si doy con la tecla, pero tiene pinta de ser algo del estilo de lo de antes... 😕

@@ -96,24 +96,11 @@ def update(self, controls, system, environment):
# self.Dalpha_Dt = (alpha - self.alpha) / system.dt
self.alpha = alpha
# self.Dbeta_Dt = (beta - self.beta) / system.dt
self.alpha = alpha
Copy link
Member Author

Choose a reason for hiding this comment

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

Aquí había otro """"pequeño""""" error 😞

@astrojuanlu
Copy link
Member

Por mi parte todo correcto, @AlexS12 ¿ya has solucionado los problemas de los que hablabas antes?

@AlexS12
Copy link
Member Author

AlexS12 commented Aug 14, 2016

Estamos trabajando en ello 📝 😭 🔫

@AlexS12
Copy link
Member Author

AlexS12 commented Aug 15, 2016

Por mi parte, listo para hacer el merge. ✈️
Habrá que ir arreglando los tests, la documentación y limpiando un poco algunas de las clases ♻️, pero podemos hacerlo desde el master de modo que el flujo de trabajo sea más fácil para la gente nueva que se ha incorporado. 😀

@AlexS12 AlexS12 merged commit e610249 into AeroPython:master Aug 15, 2016
This was referenced Aug 17, 2016
@AlexS12 AlexS12 deleted the oop-refactor branch November 19, 2017 22:00
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.

4 participants