Skip to content
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

Allow __ft__ methods to return tuples #596

Merged

Conversation

Rahulbeniwal26119
Copy link
Contributor

This Pr is regarding issue raised in fasthtml Issue

  • Added test cases to make sure existing functionality of to_xml is not broken.
  • Changing ordering of execution (ft before to_xml in recursion) make sure to convert objects to tuple representation first.
  • Suggestions are appreciated.

Thanks

Copy link

Check out this pull request on  ReviewNB

See visual diffs & provide feedback on Jupyter Notebooks.


Powered by ReviewNB

@jph00
Copy link
Member

jph00 commented Aug 10, 2024

Looks good @Rahulbeniwal26119 ! Can you change that example in the notebook into a test? You can use assert or fastcore's test_eq. Also, there's some conflicts to resolve.

@Rahulbeniwal26119
Copy link
Contributor Author

@jph00 merge conflict resolved please review

@jph00
Copy link
Member

jph00 commented Aug 11, 2024

Thank you!

@jph00 jph00 merged commit ba8bcd1 into fastai:master Aug 11, 2024
6 checks passed
@jph00 jph00 added the enhancement New feature or request label Aug 11, 2024
@jph00 jph00 changed the title fixing to_xml to parse Titled correctly for fasthtml Allow __ft__ methods to return tuples Aug 11, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants