Skip to content

  • Projects
  • Groups
  • Snippets
  • Help
    • Loading...
    • Help
    • Submit feedback
    • Contribute to GitLab
  • Sign in
haskell-gargantext
haskell-gargantext
  • Project
    • Project
    • Details
    • Activity
    • Releases
    • Cycle Analytics
  • Repository
    • Repository
    • Files
    • Commits
    • Branches
    • Tags
    • Contributors
    • Graph
    • Compare
    • Charts
  • Issues 161
    • Issues 161
    • List
    • Board
    • Labels
    • Milestones
  • Merge Requests 8
    • Merge Requests 8
  • CI / CD
    • CI / CD
    • Pipelines
    • Jobs
    • Schedules
    • Charts
  • Wiki
    • Wiki
  • Snippets
    • Snippets
  • Members
    • Members
  • Collapse sidebar
  • Activity
  • Graph
  • Charts
  • Create a new issue
  • Jobs
  • Commits
  • Issue Boards
  • gargantext
  • haskell-gargantexthaskell-gargantext
  • Merge Requests
  • !149

Merged
Opened Apr 25, 2023 by Alfredo Di Napoli@AlfredoDiNapoli
  • Report abuse
Report abuse

Fix NGrams pagination (purescript-gargantext#531)

This merge request fixes the ngrams pagination problem(s). I say "problems" (plural) because when I was working on this issue, I've also noticed another bug: not only was the number of entries returned per page incorrect, but in case of NGramElements with children, we were not sorting correctly by the number of occurrences. To give a concrete example, in my development environment previously I had this when I requested the second page for the Haskell corpus:

Screenshot_2023-04-25_at_08.24.15

This is incorrect, because the terms Ngram had an occurrences count of 128, so it should have been listed as the third entry of the first page, not the first of the second!

After my patch, both bugs have been fixed:

correct_pagination

@anoe I had to make some changes to the NGrams types, in particular now the occurrences are stored as a Set ContextId rather than a [ContextId], because my assumption here is that we cannot have duplicate context ids in the occurrences, so let me know if you think this change might be causing problems elsewhere.

IMPORTANT: While reviewing this code, it occurred to me that most of its implementation is pure, so it will be really helpful to spend some time adding more tests which will shelter us in the future from possible regressions in the way we paginate. @anoe maybe something for me to work on next week while you are away?

I haven't measured the performance impact after my changes, but I think it's important to have a correct implementation before even considering its performance, which should be comparable to the previous one anyway.

Check out, review, and merge locally

Step 1. Fetch and check out the branch for this merge request

git fetch origin
git checkout -b adinapoli/issue-incorrect-pagination origin/adinapoli/issue-incorrect-pagination

Step 2. Review the changes locally

Step 3. Merge the branch and fix any conflicts that come up

git fetch origin
git checkout origin/dev
git merge --no-ff adinapoli/issue-incorrect-pagination

Step 4. Push the result of the merge to GitLab

git push origin dev

Note that pushing to GitLab requires write access to this repository.

Tip: You can also checkout merge requests locally by following these guidelines.

  • Discussion 1
  • Commits 1
  • Pipelines 1
  • Changes 2
Assignee
Assign to
None
Milestone
None
Assign milestone
Time tracking
0
Labels
None
Assign labels
  • View project labels
Reference: gargantext/haskell-gargantext!149

Revert this merge request

This will create a new commit in order to revert the existing changes.

Switch branch
Cancel
A new branch will be created in your fork and a new merge request will be started.

Cherry-pick this merge request

Switch branch
Cancel
A new branch will be created in your fork and a new merge request will be started.