Fixes metadata propagation

[kylebgorman]

I am still baffled how we are getting a TokenList or a SentenceList without this metadata in the first place, but this fixes it and adds relevant tests.

I also fixed the issue with filtering losing metadata, making the most minimal change I could.

Closes #97.

[EmilStenstrom]

This is soo close, but I see you included a whole batch of formatting changes, making it hard to get an overview of all the changes that really matters. Is you editor configured to run black or something like that?

[kylebgorman]

This is soo close, but I see you included a whole batch of formatting changes, making it hard to get an overview of all the changes that really matters. Is you editor configured to run black or something like that?

Yes.

[kylebgorman]

Would you like a version bump on this too? This is blocking release of something else I'm working on and it wouldn't hurt to pin to >= 5.0.2.

[EmilStenstrom]

@kylebgorman I'll handle the version bump. Coverage fails because there isn't a test for the fixes to extend. Mind adding a test for that too? (Sorry for this being strict, I just don't want to mess things up for existing users...)

[kylebgorman]

I'm sorry but I don't understand how to satisfy the test coverage thing. .extend is already being tested to my knowledge.

[EmilStenstrom]

@kylebgorman Thanks for bearing with this! If I click "Details" to the right of the failing test, I get a long test run log. At the end I see that coverage failed. If I scroll up and find tox: coverage and expand that block, I get a breakdown of the lines that coverage says is missing tests. Seems it's not the whole extend function, just line 357, where a Metadata block is created if it doesn't already exist, that isn't tested. Makes sense?

[kylebgorman]

@kylebgorman Thanks for bearing with this! If I click "Details" to the right of the failing test, I get a long test run log. At the end I see that coverage failed. If I scroll up and find tox: coverage and expand that block, I get a breakdown of the lines that coverage says is missing tests. Seems it's not the whole extend function, just line 357, where a Metadata block is created if it doesn't already exist, that isn't tested. Makes sense?

I understand what your test coverage feature is saying but I don't have any intuitions what I'm supposed to do about it. My last commit has a rather silly test that might hit it, but I don't know, and the tox setup isn't working on either of my computers so I'm depending on online CI.

[EmilStenstrom]

@kylebgorman With a minor misspelling fix, it worked! I've now released it as https://pypi.org/project/conllu/5.0.2/

You can try running just coverage locally to reproduce:

• pip install coverage
• coverage run --branch -m pytest -m "not integration"
• coverage report -m --fail-under=100

[kylebgorman]

Thanks for this and for your help.

On Thu, Sep 19, 2024 at 4:25 PM Emil Stenström @.***> wrote:

@kylebgorman https://github.com/kylebgorman With a minor misspelling fix, it worked! I've now released it as https://pypi.org/project/conllu/5.0.2/

You can try running just coverage locally to reproduce:

• pip install coverage
• coverage run --branch -m pytest -m "not integration"
• coverage report -m --fail-under=100

— Reply to this email directly, view it on GitHub https://github.com/EmilStenstrom/conllu/pull/99#issuecomment-2362110939, or unsubscribe https://github.com/notifications/unsubscribe-auth/AABG4ONXARRXKSI4O5Q6WWLZXMXMLAVCNFSM6AAAAABOJ6SAVSVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDGNRSGEYTAOJTHE . You are receiving this because you were mentioned.Message ID: @.***>