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

Status of the project #5

Open
sanzoghenzo opened this issue Jan 25, 2023 · 3 comments
Open

Status of the project #5

sanzoghenzo opened this issue Jan 25, 2023 · 3 comments

Comments

@sanzoghenzo
Copy link
Contributor

Hi there,
first of all, thanks for this! After some research, I think this extension would be what I'm looking for, but I did encounter 2 issues for my use case (also found in yafg):

  • stripping the title results in a KeyError if no title is specified
  • if I don't strip the title, i got a "Figure 1: None" caption.

I would like to do a PR for this, but I was wondering what are your plans for this project...

  • do you intend to continue/resume the development or should I start a new, independent, fork?
  • which is the best branch/commit to continue your work? I see that ContentHack branch is ahead of the others, but I has many failing tests. Are they bugs to squash or there's some missing functionality still to implement?
@flywire
Copy link
Owner

flywire commented Jan 25, 2023

The ContentHack branch was working towards my vision of allowing a configurable caption for any content based on the original concept. I refactored the code so it made more sense in that context. Unfortunately, my python OOP skills are inadequate and I've stalled.

iirc:

  1. I understand a superclass is needed once the content type is determined but I couldn't figure it out
  2. two lines are written in the wrong order for tables

Of course if it worked then tests would be needed.

Any PR to resolve these or any other issues would be welcome.

@sanzoghenzo
Copy link
Contributor Author

That's wonderful to hear (well, read 😅 )!

OK, now I understand what you tried to do, and while my PR fixes all the tests, they aren't really good from an OOP perspective. But at least we now know that it works and that a refactor should always pass the tests.

We can make the various content types subclasses of the CaptionTreeprocessor and register each one in the extendMarkdown method, so that the parent class only has common code and the children only have specialized code.

I would gladly help you do this, let me know if you want to merge the current PR first or if it's ok to go all in with the overhaul.

PS: I already did some more refactorings, mainly to make it more compliant with the google style guide (variable/method/class naming) and to switch to pytest (just a personal preference, it avoids keeping a useless class around), but let's take it one step at a time 😉

@hendrikp
Copy link

hendrikp commented Mar 4, 2023

Small note in the wiki on page https://github.com/flywire/caption/wiki/Development#unittest

the line
python -m unittest -v test_caption.captionTestCase

should be replaced by
pytest --cov

As the testing framework was changed.

@flywire flywire mentioned this issue Mar 16, 2024
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

No branches or pull requests

3 participants