Skip to content

Conversation

kubaneko
Copy link
Contributor

@kubaneko kubaneko commented Jul 1, 2022

This PR is for implementing #986 as an GSoC project. Mentored by @gbaz .

I will correct the error I made in the change.

@gbaz gbaz changed the title PageRank PackageRank Jul 1, 2022
@kubaneko kubaneko marked this pull request as draft July 3, 2022 20:32
Copy link
Member

@ysangkok ysangkok left a comment

Choose a reason for hiding this comment

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

I think you can avoid these partial functions by using NonEmpty. See comments.

versionList = sortBy (flip compare)
$ map (pkgVersion . package . packageDescription) (pkgDesc <$> pkgs)
packageEntr = do
tarB <- packageTarball tarCache . head $ pkgs
Copy link
Member

Choose a reason for hiding this comment

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

Avoidable partial function

return (temp <> versS <> codeS <> authorScore maintainers pkg)

where
pkg = packageDescription <$> pkgDesc $ last pkgs
Copy link
Member

Choose a reason for hiding this comment

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

Avoidable partial function

constructItem :: [PkgInfo] -> IO (PackageName, PackageItem)
constructItem pkgs = do
let pkgname = packageName pkg
pkg = last pkgs
Copy link
Member

Choose a reason for hiding this comment

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

Avoidable partial function

total
. (<>) (rankPackagePage pkgD)
<$> rankIO versions recentDownloads maintainers docs env tarCache pkgs
where pkgD = packageDescription $ pkgDesc $ last pkgs
Copy link
Member

Choose a reason for hiding this comment

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

Avoidable partial function

False -> do
index <- queryGetPackageIndex
let pkgs = PackageIndex.lookupPackageName index pkgname
case pkgs of
Copy link
Member

@ysangkok ysangkok Aug 16, 2022

Choose a reason for hiding this comment

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

this can case on Data.NonEmpty.nonEmpty pkgs instead. Then the Just nonEmptyList case can pass that list on to constructItem

@kubaneko
Copy link
Contributor Author

@ysangkok

Thank you for the comments

In my last two commits I fixed all the partial functions in PackageRank.

The partial function in PackageList is in my opinion better left as is since it was that way even before my changes and on line 256 using NonEmpty would lead to Maybe PackageIndex. Which is not something that is wanted?

@kubaneko kubaneko marked this pull request as ready for review August 26, 2022 21:52
@kubaneko
Copy link
Contributor Author

At this moment PackageRank should be structure-wise complete. Some parameters will need to be adjusted and there may be some bugs.

Caching some data (ex. freshness for dependencies would require parsing all their PkgInfo (not included)) is missing.

UI is a bit arbitrary so that might also change.

@kubaneko
Copy link
Contributor Author

kubaneko commented Sep 2, 2022

image

UI at the moment @ysangkok

@nikita-volkov
Copy link

What's blocking us from having this merged? Looks like a great feature to have.

@kubaneko
Copy link
Contributor Author

kubaneko commented May 7, 2024

The indirect dependencies feature was merged and it would be nice to use it here, adding a cache for the package values would be good and the parameters are somewhat arbitrary at the moment.

I plan to get this into mergeable state in June.

@ysangkok
Copy link
Member

@kubaneko Good, that's great to hear.

@kubaneko
Copy link
Contributor Author

kubaneko commented Sep 22, 2025

I rebased and added some version of #1082 to PackageRank. I tested this by mirroring 600 package from hackage and the numbers it gives look reasonable.

The performance implications are unclear to me, since I do not have the download speed to mirror the whole hackage but it was not too bad (I am unsure if adding indirect dependencies is a problem).

There is definitely still work that can be done:

  • in freshnessScore, freshness of dependencies should be taken into account
    • this would probably require a cache
  • cache would also be a good idea since Package rank does do some computation
  • tuning the parameters to Haskell would be nice
  • refactoring the code a bit would help

Currently I would like if someone with a hackage mirror would try what the slowdown is. If it is not noticable than I propose to merge this otherwise I will get to making the cache.

Any more comments/suggestions are of course welcome

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.

3 participants