@lucasmoura lucasmoura force-pushed the recommendation_file branch 2 times, most recently from b0904ad to 83a5c2b Compare June 13, 2016 19:38
@lucasmoura
Copy link
Contributor Author

I have added some tests for invalid recommendation XML files

@ximion
Copy link
Owner

ximion commented Jun 13, 2016

Can you please give me a complete exmplanation why you need all of that new XML stuff?
If the AppRecommender runs as independent tool, why can't it just throw out a list of AppStream IDs that libappstream can then read and flag specific components as "recommended"?
All the things you write seem to be a bit overcomplicated for this task, TBH.

@lucasmoura
Copy link
Contributor Author

Yes, sure. The reason I am creating the because tag is an attempt to allow users to better understand why a recommendation is being done. For example, suppose that I just use a list of AppStream ids as a recommendation file. If any application wants to use AppStream to show these data, it will only show the recommended packages for the user, but it will not be possible to show why that package was recommended in the first place.

Therefore, I believe that using the because tag can make the recommendation more trustworthy for the user.

But if you think that tag should not be treated on AppStream or it will not be as useful, I can remove it with no problem :)

@aleixpol
Copy link
Collaborator

I think that what @ximion means is that the format itself could be reduced somewhat. In the end, we're just offering a pair of packages such as. It's about offering pairs such as gdb recommends gcc and vim+x11 recommends kate. Finding ways to easily offer those could help adoption.

FWIW, do recommendations be directional? Does emacs recommends vim inherently mean the opposite?

@lucasmoura
Copy link
Contributor Author

@aleixpol True, that makes sense. I will try to think of a better representation for a recommendation file.

About the second question, I don't know for sure if the recommendations are directional. Since the user profile is a combination of the most significant terms of the user manual installed packages, I cannot answer for sure if swapping a recommended package for an installed one, would then make the recommender system to recommended the installed package. But again, I may be wrong on that, since I have never looked at this situation closely.

@ximion I think I have misunderstood your comments, I believe you are not talking about the because tag, but the new code I am adding to the feature. Basically, the only new feature I have added is the one that checks if a recommendation object actually holds a package indexed by AppStream. The other code I have written is just to make the test suite more robust.

@ximion
Copy link
Owner

ximion commented Jun 14, 2016

@lucasmoura There are basically two fundamental things I am criticising:

  • The code changes you do are massive and duplicate most of the XML and AppStream parsing logic, while we already do have logic to parse AppStream-style XML data and enhance existing data with it.
    For example, if you have an AppStream component with ID "org.example.foobar.desktop" defined, and then have the generator read XML like:

    <component type="desktop">
    <id>org.example.foobar.desktop</id>
    <recommendation>
      <because>XYZ</because>
    </recommendation>
    </component>

    The XML reader you yet a recommended=True flag on the original component type, and apps could show that information immediately. It would also greatly reduce code duplication.
    Right now, you create an AsRecommendation class that is separate, and I am not sure if that is actually needed. I am also not sure if we need information on why something was recommended at all, because users in software-centers don't really care about that. In case someone cares, they could ask apprecommender for that intel directly.

  • I am not yet sure about the whole because-of stuff and the general design. It would be nice if you could outline who is generating what and what exactly you have in mind how this thing should work. I can guess from your code, but that's really not enough ;-) By doing so, you'd also safe yourself from doing work which doesn't get merged - I really hate to see people putting lots of working into new code and then not being able to merge a PR because it doesn't fit the design of the application.
    So, what I would like to see is something like "AppRecommender reads list of packages, resolves them to Components via libappstream, produces a list of recommended Component-IDs based on that data and writes a list of them and why they were recommended to /var/cache/app-info/recommendations (or to some path in $HOME). AppStream itself will read that file, mark recommended applications/components as such and allow software-center GUIs to display that information. AppStream should allow SC apps to display why something was recommended by parsing the machine-readable AppRecommender XML and presenting it to the SCs."

@lucasmoura
Copy link
Contributor Author

@ximon Now I have completely understood you point. Also, I am really sorry for taking that long to understand it. Yes, using the component XML infrastructure to make a recommendation would definitely be better. I was actually studying ways to solve the problem of code duplicate, but I think that approach solves that problem nicely. Therefore, I will change my code for this new format.

For the second part. Yes, I will make a step by step guide of how this data will be generated and consumed. I strongly believe that I will make that guide tonight.

But thanks again for the patience and sorry one more time for my delay on understanding your point.

@lucasmoura
Copy link
Contributor Author

lucasmoura commented Jun 15, 2016

@ximion This is the step by step guide on how I believe the whole recommendation feature would work.

  1. During the package installation of AppRecommender, it will create an user profile based on the user manual installed packages.For the context of software centers, the manual installed packages will be filtered to allow only AppStream componentes to be used.

  2. After that, AppRecommender will perform a recommendation and generate a XML file containing the component being recommended and the user components that generated that recommendation.

  3. The XML file would be similar as the component XML file, however, it will only possess an id and recommendations tag. The id tag would hold the actual package being recommended and the recommendations tag would be used to store the packages that generated the id recommendation. AppRecommender will be responsible to provide both informations.

The recommendation file would them something like this:

<component type="recommendation">
<id>org.example.foobar.desktop</id>
<recommendation>
  <because>XYZ</because>
</recommendation>
</component>
  1. AppRecommender will then place this XML file on /usr/share/app-info/recommendations

  2. After that, this file will be updated via a crontab, which will run an AppRecommender command to update that XML file. This command would run on a weekly basis.

  3. Once the file is in the proper location, AppStream will be able to read the file and loads all the recommended components on memory.

  • (I still don't know if it is better for AppStream to always read this file and create the objects dinamically, or if I should store them in a Xapian database)
  1. If there are more than one file it the /usr/share/app-info/recommendations, AppStream would read the most recent file.

  2. AppStream method would them be used to parse the recommendations and loads them on memory as components. It would be similar to the method as_database_get_all_components.
    That method would use the original AsMetadata and AsXMLData to read the file. Therefore,
    all the returned objects would be components one but with a recommendation component type.

The XMLParser would need to receive a flag that allows it to look for the recommendation tag.
(I think that the Software Center application should be the one that search for a *desktop component related to recommendation component. But I may be wrong on that matter.)

  1. A Software Center to use this feature would them use this feature to load the recommended components and display then to the users.

  2. The recommendation objects would contain the package being recommended and the packages that generated that recommendation.


That is basically my general idea on how that whole integration will work. However, I believe that if the because tag will be present on the XML file, the AsRecommendation class would be needed, I could use the pkgnames variable on AsComponent to hold that information, but I don't think it is a good idea. But since you stated that Software Center users will not care for that additional information at all, maybe it is best to drop it.

@lucasmoura lucasmoura force-pushed the recommendation_file branch 5 times, most recently from 4e797ad to ef9cb32 Compare June 19, 2016 21:23
@lucasmoura
Copy link
Contributor Author

@ximion I have implemented a new solution using now AppStream already implemented methods to parse the recommendation file.

The feature works as specified in my the guide I have provided. Basically, now I am using the as-data-pool to parse the recommendation files. Furthermore, the AsRecommendation object now only serve the purpose of retrieving the recommendation components and nothing else.

I have also removed the meta information about why a recommendation was generated and just used the list of component ids as a recommendation file.

I still don't know if the verification if the recommendation file holds a valid component should be performed on the AsRecommendation object, or if it should be done by the user who wants to use that feature.

@ximion
Copy link
Owner

ximion commented Jun 21, 2016

To:

  1. Sounds good to me - although it would probably be useful to operate directly on AppStream-IDs in $future, because these include software from bundling sources as well.

  2. What concept are you referring to with "user components"?

  3. A component-type "recommendation" doesn't make sense, because a component type is always a specific type of installable software. Recommendation is a property of a component. If we decide to drop the why-is-it-recommended part, Apprecommender could also simply output a plaintext file listing AppStream-IDs - those could be read faster. We would then set a recommended? boolean in AsComponent for recommended apps.

4 and 5) Sounds good

  1. AppStream will read the XML and use Xapian as cache - that's the basic concept. I will also only update the cache if something has changed.

  2. I don't like that "most recent file" logic - can't there just be one file? Why do you expect multiple files to be there? Also, the default mode of action would be to read all files use the information there. If a file is no longer relevant, it should be deleted.

  3. Recommendation component-type: Nonononono, see 3) - a recommendation is a property of an AsComponent.

  4. Jup

  5. You mean the component-id being recommended, not the package, right?


Independent from that: Will you be at Debconf? If so, we can discuss this there and work on it.

@ximion
Copy link
Owner

ximion commented Jun 21, 2016

Calling the software-center developers: @hughsie @aleixpol @tintou @ikeydoherty
Please don't read the whole bug report, I am highlighting you for this very simple question:
Would you in your software-center applications, when you display "featured" or "recommended" apps (with the recommendations based on what else the user has installed) also display why these apps were recommended? (As in "Vim is recommended because you installed gcc, devhelp and cmake")

I would say this information is nothing which the user cares much about, because it's nothing which is actionable and is purely for the people who are curious about how the recommendations work. If they get "bad" recommendations, that info won't help, if they get good recommendations, they won't care. Also, the info can still be retrieved via CLI tools, and might clutter software center UIs, which already have to show quite a lot of information in an appealing way.

So, what do you think? I am very reluctant to let the complex "X because Y and not Z" logic into the spec and have it implemented in libappstream. With my maintainer's hat on, I think this will bite me later, so I would not want this feature in AppStream itself. Of course, my opinion on it will change if you say you want to expose this feature to end users and would like to have it in SC UIs.

@lucasmoura
Copy link
Contributor Author

@ximion

Answering a couple of questions:

  1. The "user components" would be the user packages that generated the recommendation and that are also indexed on AppStream.

  2. Fair enough. I created the recommendation type to allow the as_component_is_valid to work properly, but now I see that it doesn't make sense in the context of a component. Using the recommended? variable on a component would work way better.

  3. Okay

  4. Maybe I have overthought that step. I was thinking if the user has more than one recommender application that would like to integrate with AppStream. But allowing a single file would be a better approach.

  5. Yes, my mistake there.


I will be attending DebConf this year, and yes, it will be great if we could discuss and work on this there :)

@colomar
Copy link

colomar commented Jun 21, 2016

@ximion Answering for Discover because I'm the one responsible for its user interface: While I would not display the "why" by default (to reduce clutter), I would want to make it easily accessible on demand. The reason being that users may gauge the usefulness of a recommendation based on its source.
If an application is recommended to me based on one which I use regularly, I may be more likely to heed it compared to one I got based on some app I installed just to try it out or for some specific task and have rarely - if ever - used it since (assuming people do not uninstall applications they do not use)

@hughsie
Copy link
Collaborator

hughsie commented Jun 22, 2016

So, I see this useful in the same way I see extends being used, e.g.

<component>
   <id>gimp.desktop</id>
  <recommend>mypaint.desktop</recommend>
  <recommend>krita.desktop</recommend>
</component>

I have no idea why you'd want to encode the "why" in the data whatsoever. If we do, something like:

<recommend reason="same-developer">colorhug-client.desktop</recommend>

...could work i guess, and would allow us to do things like the play store.

@danirabbit
Copy link

Hm I'm not sure why you'd want to hate code this into the spec. It seems that would mean apps developers have to be aware of every other possible package.

But regarding why that info is useful, as says above the user might gauge the usefulness of the recommendation based on their feelings of another app. But also, this information is valuable if a feature exists for the user to mark an app to not be used for recommendations. Think Amazon. I'm getting recommendations that I think are not relevant and I want to explain to the computer why I think they're not relevant.

@ximion
Copy link
Owner

ximion commented Jul 7, 2016

Notes from Debconf:
Allow upstream metainfo files to include recommendations in form of

<component type="desktop">
  <id>org.example.Foo.desktop</id>
  <name/>
  <summary/>
  <description/>
  <suggests type="upstream">
    <id>org.example.Bar.desktop</id>
    <id>...</id>
  </suggests>
</component>

The type might be omitted, and metainfo files are only allowed to include type "upstream" suggestions.

Allow distro metadata files to enhance existing components, add a "heuristic" type for AppRecommender:

<components>
  <component type="partial">
    <id>org.example.Foo.desktop>
    <suggests type="heuristic">
      <id>org.example.XYZ</id>
      ...
    </suggests>
  </component>
  ...
</components>

This way, software centers have basic informtion on where a recommendation is coming from, and also AppRecommender doesn't need to invent its own format and can just use standard AppStream metadata (as soon as this spec extension is standardized).

@lucasmoura
Copy link
Contributor Author

If the package being recommended is not the one receiving the suggests tag, I think that there might be some problems. A recommendation in AppRecommender is generated by the combination of multiple packages. Therefore a single package recommending others may not be a good representation of the process.

Maybe the suggests tag can turn into something like a suggested-by tag and if a component has this tag populated it can be assumed that it is a recommendation. But I don't know if that would the best approach here.

@hughsie
Copy link
Collaborator

hughsie commented Jul 7, 2016

I think it's fine the AppStream "app A suggests app B" is encoded into the file. I don't think we want to put the internal data structure or exact reason in the output data.

@lucasmoura
Copy link
Contributor Author

Fair enough. I am only a little bit uncertain about making AppRecommender produce a result such as "app A recommends app B", because in reality would be something like "app A, B, C and D recommends E". I am afraid that by doing so, considering only one package, the recommendation produced by AppRecommender may be a little misleading.

But I can check that out and see if it is true.

@lucasmoura lucasmoura force-pushed the recommendation_file branch 2 times, most recently from e1bce6c to a12c5bf Compare July 19, 2016 23:37
@lucasmoura
Copy link
Contributor Author

@ximion I thing I have created the necessary changes to allow AppStream to handle suggestions metadata. However, I am not sure if I am testing the best way I can or if the tests are in the right file. If there is still anything wrong with my solution, please let me know :)

@lucasmoura
Copy link
Contributor Author

I just forgot to say that, if everything is alright with this solution, I can start working on its documentation as well.