Skip to content

Conversation

@Pratichhya
Copy link
Collaborator

Testing for better visualization and management of pages as a possible solution to increasing data collections.

@github-actions
Copy link

github-actions bot commented Sep 30, 2023

PR Preview Action v1.4.4
🚀 Deployed preview to https://eu-cdse.github.io/documentation/pr-preview/pr-311/
on branch gh-pages at 2023-10-06 11:18 UTC

@Pratichhya Pratichhya closed this Sep 30, 2023
@Pratichhya Pratichhya deleted the test_datavisualization branch September 30, 2023 11:35
@Pratichhya Pratichhya restored the test_datavisualization branch September 30, 2023 11:35
@Pratichhya Pratichhya reopened this Oct 3, 2023
@Pratichhya Pratichhya closed this Oct 3, 2023
@Pratichhya Pratichhya reopened this Oct 6, 2023
@Pratichhya
Copy link
Collaborator Author

Hi @jdries

Follow-up on #270
I've identified a few pain points in the current display, and I've come up with a listing solution to improve the visualization of the entire collection. While https://eu-cdse.github.io/documentation/pr-preview/pr-311/Data.html renders as expected, https://eu-cdse.github.io/documentation/pr-preview/pr-311/Data/SentinelMissions/Sentinel2.html still has some issue. I identified the cause of it but still looking for a solution. You can see in the images below how it could look (I've tested it locally).

Also, at the moment, the collection files are generated on-fly, and it stays there, so I am not sure if there could be better ways.

However, before I continue further, could you please share your feedback and suggestions on whether this visualisation solution could be an option?

Furthermore, I think it's important to address PR1, PR2, or any relevant PRs to prevent merge conflicts, as these changes will involve rearranging the files.

sentinel2
sentinel3

@jdries
Copy link
Collaborator

jdries commented Oct 6, 2023

For me this already looks like a good improvement over the previous approach, as we fit more info in a single view. Of course, UX experts like @JanssenBrm or @soxofaan may also have some thoughts?
Maybe one thing is that we can order tiles based on level in descending order rather than ascending because level-0 is often the least useful level for a user.
How does this work from a code point of view, is it maintainable? (Or does it perhaps require a lot of custom code?)

@Pratichhya
Copy link
Collaborator Author

For me this already looks like a good improvement over the previous approach, as we fit more info in a single view. Of course, UX experts like @JanssenBrm or @soxofaan may also have some thoughts? Maybe one thing is that we can order tiles based on level in descending order rather than ascending because level-0 is often the least useful level for a user.

Yes, ordering the tiles was possible; I tried based on time/alphabet but didn't implement it here. It is now based on file naming. However, more might be possible.

How does this work from a code point of view, is it maintainable? (Or does it perhaps require a lot of custom code?)

No, if we figure out what to change in cards/listing, it was only a few lines at the top since all text comes from metadata so easily manageable. The only tricky now is figuring out some of these issues with rendering.

@soxofaan
Copy link
Collaborator

soxofaan commented Oct 9, 2023

like @JanssenBrm or @soxofaan may also have some thoughts?

Looks indeed better. Not much to add at this point. Maybe adding the thumbnail image when available?

@Pratichhya
Copy link
Collaborator Author

like @JanssenBrm or @soxofaan may also have some thoughts?

Looks indeed better. Not much to add at this point. Maybe adding the thumbnail image when available?

I agree and actually, with a default setting the first image appears as a thumbnail, so it should be possible to modify(at the moment the icon comes up as image)

@Pratichhya
Copy link
Collaborator Author

If everyone agrees @jdries @soxofaan @JanssenBrm ,

I would suggest first finalizing the actions for existing data-related PRs(either merge or close) esp #270 and #216, since they can create merge conflict later.
Then subdividing this PR to:

  1. First display the introduction section and rearrange the existing files for all 3 sections
  2. Create the same display for the rendered collection.

import sys
import os
sys.path.append('..')
Copy link
Collaborator

Choose a reason for hiding this comment

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

This sys.path.append('..') looks strange to me: you want to be able to import custom files outside of the repository?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

yes, I tried some other ways, too, but since it was being rendered and function files were in different locations, it caused an issue. Maybe rearranging in order for these function files might also be needed and should be taken into consideration.

Copy link
Collaborator

Choose a reason for hiding this comment

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

sys.path.append('..') is a brittle solution as it assumes that you are working from a certain directory. I guess there must be some quarto config to just specify addtional paths to search for dependencies you want to import, but I don't know quarto well enough to figure that out at the moment.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Sure, I can look into this aspect; there might be a way within Quarto.

@Pratichhya Pratichhya marked this pull request as draft October 31, 2023 11:55
@soxofaan
Copy link
Collaborator

soxofaan commented Oct 9, 2024

Is this PR still relevant? I see there are a lot of conflicts with main branch, so maybe it's best to start over.

In any case I'm going to remove the pr-preview build for this PR in the context of #633

soxofaan added a commit that referenced this pull request Oct 9, 2024
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.

4 participants