Skip to content

Fix to_dict() #25

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Draft
wants to merge 5 commits into
base: master
Choose a base branch
from
Draft

Conversation

SillyFreak
Copy link

@SillyFreak SillyFreak commented Jul 19, 2025

Fixes #23, based on #24 for obvious reasons

Note that test_core/test_grades.py is currently failing, even before this PR -- maybe something to do with Moodle 5.0 compatibility?

This PR reimplements to_dict() and changes some behavior that was not depended upon:

  • Due to recursion, the function could previously accept any value and also accept a name parameter. The actual recursion is now in a nested function, so I

    • removed the name parameter.
    • require that the passed data are a dictionary. A HTTP request always has named parameters, only the nested values will eventually not be dicts

    This means that you can't pass a @attr.s class directly to the function. You can see the commented regressed test case here:

    # attr class
    # result = to_dict(MyAttrClass(x=1, y=2))
    # assert result == { 'x': 1, 'y': 2 }

    If you have concerns about this, I can of course also handle this case.

  • I didn't know what the purpose of these lines were so I didn't preserve them:

    # Check if data required name prefix
    if hasattr(data, "name"):
    out_key += f"[{getattr(data, 'name')}]"

    because of this, I have marked this PR as a draft. It hasn't caused regressions, though.

You can take a look at cd3d959 to see what previously failing test cases now succeed. Additionally, in 2e552b8 I edited the test case to fail as described in #23.

Beyond that one calendar test case, I have not tested that the new behavior matches PHP's expectations of form encoded data, so I'd ask you to look over the test cases.

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.

[BUG 🐞] to_dict() does not handle top-level single structures correctly.
1 participant