search

desklab / gcampus

GewässerCampus is an educational project on limnology aiming to provide students the possibility to analyse, publish and discuss results of self-conducted measurements of water-quality in rivers and lakes.
https://gewaessercampus.de
GNU Affero General Public License v3.0
3 stars 1 forks source link

review TODOs and clean up code #59

Closed ax-sc closed 2 years ago

ax-sc commented 2 years ago
ax-sc commented 2 years ago

marked some items as done which are handled by #62 .

3j14 commented 2 years ago

In the following line we have a TODO that I'm not sure about:

https://github.com/desklab/gcampus/blob/72429a7477fd4393724ae164e72d08abb3b9bc69/gcampus/core/templates/gcampuscore/sites/detail/water_detail.html#L141

I could of course implement this, but I'm unsure whether we actually want this. For most waters it should be clear which measurements belong to the water that is currently display just by their position on the map. Do we still want to hide all other measurements?

ax-sc commented 2 years ago

I think the main benefit would be with large waters, where both measurements of the same water and measurements of different waters are clustered together at the default zoom level. The original intention was to make it easier to discover similar measurements at the same water, but doing this on the map will most likely become superfluous once we have implemented the similar measurement section we already discussed when planning the implementation of the water quality indices.

ax-sc commented 2 years ago

just had another look on the TODOs which are still in the code. Maybe we should exclude the ones which are related to the documents and the printed layout from this issue and handle them separately (except for the calculation of the bounding box), possibly even dedicating a complete minor version to improving the documents (and cleaning up their code) at some point?

3j14 commented 2 years ago

Maybe we should exclude the ones which are related to the documents and the printed layout from this issue and handle them separately [...], possibly even dedicating a complete minor version to improving the documents [...] at some point?

Sound good!

Other than that, I went through all the TODOs now. All that is left is the calculation of the bounding boxes. Will do that later :)

3j14 commented 2 years ago

Fixed the boundary box with 6736305. I used the following GeoDjango function: https://github.com/desklab/gcampus/blob/67363052d03ce1b71fc59d78fef8c318c886a266/gcampus/documents/views/print.py#L121

Note that the bounding box is invalid if only one measurement is in the list. The code for the static map API catches this and reverts to using the single measurement as the center argument.

3j14 commented 2 years ago

Sorry, code cleanup is not done yet 😄 But should not take long, most packages are already done.