Skip to content
This repository was archived by the owner on Aug 9, 2024. It is now read-only.

Conversation

@halfak
Copy link
Member

@halfak halfak commented Aug 22, 2019

This implements a nice structure for session orientation using a helper function called "list_of_tree" that takes a DependencySet and iterates through it converting all children and maintaining the structure of the tree. A cache is used to keep track of what dependencies have already been converted so that they can be re-used.

  • session: Session
    • revisions: Revision
      • diff: Diff
      • page: Page
        • namespace: Namespace
        • creation: Revision
      • parent: Revision
        • user: User
    • user: User
      • info: UserInfo
      • last_revision: Revision
        • page: Page
        • namespace: Namespace

@halfak halfak force-pushed the session_orientation branch 2 times, most recently from 90cef5c to 41c409e Compare August 26, 2019 21:23
Copy link
Member

@accraze accraze left a comment

Choose a reason for hiding this comment

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

There are a couple of failing tests due to some refactoring that need fixing, but overall LGTM!
https://travis-ci.org/wikimedia/revscoring/builds/582717322#L794

@halfak halfak force-pushed the session_orientation branch 5 times, most recently from c6cc8d8 to 66b5e35 Compare October 4, 2019 20:41
Copy link
Member

@accraze accraze left a comment

Choose a reason for hiding this comment

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

Looks great so far, everything seems well documented and tested.

@halfak halfak force-pushed the session_orientation branch from da8d20b to 04bb9df Compare November 7, 2019 21:50
Copy link
Member

@accraze accraze left a comment

Choose a reason for hiding this comment

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

I like the @DependentSet.meta_dependent decorator, although not sure if it will be a ton of work to find all functions that it should be added to. If you aren't worried about that then I would say go ahead with applying this to the language based features.

Also it looks like there is a problem with docs checker on Travis:

revscoring/features/bytes/session_oriented.py:docstring of
revscoring.features.bytes.Session.revisions:1:py:class reference target not found: 
revscoring.datasources.meta.expanders.list_of`(:class:`~revscoring.features.bytes.Revision

if not hasattr(method, "meta_dependent"):
pass
else:
list_of_meta_method = meta_list_of_ify(
Copy link
Member

Choose a reason for hiding this comment

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

👍

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants