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

Add first implementation of papis-doctor #421

Merged
merged 80 commits into from
Dec 20, 2022

Conversation

alejandrogallo
Copy link
Member

As we discussed some time ago, that would be nice to have a papis doctor
functionality.o

I have prepared a first implementation of a candidate.
In principle one implement different tests
and then queries the library like on all other commands.

Right now there are two tests implemented,
one that looks for missing keys and another one that
checks if the files of the documents actually exists.
Here is an output from my library.

papis doctor --all
WARNING:doctor:12 errors found
keys author /home/gallo/Documents/papers/3d7c4e4778f26e1c66684a11f604b9f4
keys author /home/gallo/Documents/papers/b30815699c8c34bf2fdd472cdc1f56c9
keys author /home/gallo/Documents/papers/5296700afbfb46da3e9361774ad9440a
keys author /home/gallo/Documents/papers/c666475c4b79a6216aa04e1914e72f15
keys author /home/gallo/Documents/papers/c1d2e472a175482dfa30670b30eb583f
keys author /home/gallo/Documents/papers/56c9ea78eb34406e61a52631c09f992f
files /home/gallo/Documents/papers/e31d853be4f5f11162f3c8b2abb13a2c-bassani-f.-and-inch/position-of-the-bassani-f.-and-inchauspe-n..pdf /home/gallo/Documents/papers/e31d853be4f5f11162f3c8b2abb13a2c-bassani-f.-and-inch
keys ref /home/gallo/Documents/papers/5df2864bdb8eb62891a8dc229eec1032-kaldor-uzi
keys title /home/gallo/Documents/papers/05e50e4ea461ea527851f947086c9d5b
keys author /home/gallo/Documents/papers/05e50e4ea461ea527851f947086c9d5b
keys ref /home/gallo/Documents/papers/05e50e4ea461ea527851f947086c9d5b
files /home/gallo/Documents/papers/1db30a88d9ad08ebbf76ee84bcc295f6-pederson-mark-r.-an/improved-theoretical-methods-for-studies-of-defects-in-insulators-application-to-the-pederson-mark-r.-and-klein-barry-m.pdf /home/gallo/Documents/papers/1db30a88d9ad08ebbf76ee84bcc295f6-pederson-mark-r.-an

For library writers I've also implemented a --json exporter

>> papis doctor --all --json | jq
[
  {
    "msg": "author",
    "path": "/home/gallo/Documents/papers/3d7c4e4778f26e1c66684a11f604b9f4",
    "name": "keys",
    "suggestion": "\n                 papis update --doc-folder /home/gallo/Documents/papers/3d7c4e4778f26e1c66684a11f604b9f4\n                 "
  },
  {
    "msg": "author",
    "path": "/home/gallo/Documents/papers/b30815699c8c34bf2fdd472cdc1f56c9",
    "name": "keys",
    "suggestion": "\n                 papis update --doc-folder /home/gallo/Documents/papers/b30815699c8c34bf2fdd472cdc1f56c9\n                 "
  },
  {
    "msg": "author",
    "path": "/home/gallo/Documents/papers/5296700afbfb46da3e9361774ad9440a",
    "name": "keys",
    "suggestion": "\n                 papis update --doc-folder /home/gallo/Documents/papers/5296700afbfb46da3e9361774ad9440a\n                 "
  },
  {
    "msg": "author",
    "path": "/home/gallo/Documents/papers/c666475c4b79a6216aa04e1914e72f15",
    "name": "keys",
    "suggestion": "\n                 papis update --doc-folder /home/gallo/Documents/papers/c666475c4b79a6216aa04e1914e72f15\n                 "
  },
  {
    "msg": "author",
    "path": "/home/gallo/Documents/papers/c1d2e472a175482dfa30670b30eb583f",
    "name": "keys",
    "suggestion": "\n                 papis update --doc-folder /home/gallo/Documents/papers/c1d2e472a175482dfa30670b30eb583f\n                 "
  },
  {
    "msg": "author",
    "path": "/home/gallo/Documents/papers/56c9ea78eb34406e61a52631c09f992f",
    "name": "keys",
    "suggestion": "\n                 papis update --doc-folder /home/gallo/Documents/papers/56c9ea78eb34406e61a52631c09f992f\n                 "
  },
  {
    "msg": "/home/gallo/Documents/papers/e31d853be4f5f11162f3c8b2abb13a2c-bassani-f.-and-inch/position-of-the-bassani-f.-and-inchauspe-n..pdf",
    "path": "/home/gallo/Documents/papers/e31d853be4f5f11162f3c8b2abb13a2c-bassani-f.-and-inch",
    "name": "files",
    "suggestion": "\n                  papis edit --doc-folder /home/gallo/Documents/papers/e31d853be4f5f11162f3c8b2abb13a2c-bassani-f.-and-inch\n                  "
  },
  {
    "msg": "ref",
    "path": "/home/gallo/Documents/papers/5df2864bdb8eb62891a8dc229eec1032-kaldor-uzi",
    "name": "keys",
    "suggestion": "\n                 papis update --doc-folder /home/gallo/Documents/papers/5df2864bdb8eb62891a8dc229eec1032-kaldor-uzi\n                 "
  },
  {
    "msg": "title",
    "path": "/home/gallo/Documents/papers/05e50e4ea461ea527851f947086c9d5b",
    "name": "keys",
    "suggestion": "\n                 papis update --doc-folder /home/gallo/Documents/papers/05e50e4ea461ea527851f947086c9d5b\n                 "
  },
  {
    "msg": "author",
    "path": "/home/gallo/Documents/papers/05e50e4ea461ea527851f947086c9d5b",
    "name": "keys",
    "suggestion": "\n                 papis update --doc-folder /home/gallo/Documents/papers/05e50e4ea461ea527851f947086c9d5b\n                 "
  },
  {
    "msg": "ref",
    "path": "/home/gallo/Documents/papers/05e50e4ea461ea527851f947086c9d5b",
    "name": "keys",
    "suggestion": "\n                 papis update --doc-folder /home/gallo/Documents/papers/05e50e4ea461ea527851f947086c9d5b\n                 "
  },
  {
    "msg": "/home/gallo/Documents/papers/1db30a88d9ad08ebbf76ee84bcc295f6-pederson-mark-r.-an/improved-theoretical-methods-for-studies-of-defects-in-insulators-application-to-the-pederson-mark-r.-and-klein-barry-m.pdf",
    "path": "/home/gallo/Documents/papers/1db30a88d9ad08ebbf76ee84bcc295f6-pederson-mark-r.-an",
    "name": "files",
    "suggestion": "\n                  papis edit --doc-folder /home/gallo/Documents/papers/1db30a88d9ad08ebbf76ee84bcc295f6-pederson-mark-r.-an\n                  "
  }
]

Also as you see in the json output, I thought maybe it's good
to have a suggestion function per test checked,
in text output there is the --suggest option

> papis doctor --suggest --all

...

keys ref /home/gallo/Documents/papers/05e50e4ea461ea527851f947086c9d5b
Suggestion:

                 papis update --doc-folder /home/gallo/Documents/papers/05e50e4ea461ea527851f947086c9d5b
                 

files /home/gallo/Documents/papers/1db30a88d9ad08ebbf76ee84bcc295f6-pederson-mark-r.-an/improved-theoretical-methods-for-studies-of-defects-in-insulators-application-to-the-pederson-mark-r.-and-klein-barry-m.pdf /home/gallo/Documents/papers/1db30a88d9ad08ebbf76ee84bcc295f6-pederson-mark-r.-an
Suggestion:

                  papis edit --doc-folder /home/gallo/Documents/papers/1db30a88d9ad08ebbf76ee84bcc295f6-pederson-mark-r.-an

I would say there is now the time to discuss what would be the
intended usage and the ergonomic use of the command.

Also, the code is quite straightforwardly implemented I think.

Copy link
Collaborator

@alexfikl alexfikl left a comment

Choose a reason for hiding this comment

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

Left a couple of nitpicks, but overall this works for me as advertised! Even found a document without a title 🎉

The only higher level suggestion I would have is to call these things checks instead of tests? i.e. REGISTERED_CHECKS, etc. Sounds a bit less CS-y maybe..

papis/commands/doctor.py Outdated Show resolved Hide resolved
papis/commands/doctor.py Outdated Show resolved Hide resolved
alejandrogallo and others added 2 commits November 20, 2022 23:33
Co-authored-by: Alex Fikl <alexfikl@gmail.com>
Co-authored-by: Alex Fikl <alexfikl@gmail.com>
papis/commands/doctor.py Fixed Show fixed Hide fixed
@alejandrogallo
Copy link
Member Author

Yes I agree, i changed the names to check in all related parts,
test is a very loaded word.
So you think that the workflow is acceptable? I'm not sure this is the most ergonomic.
I would also like to ask if someone has a couple of more ideas for useful checks,
like making a check about maybe repeated refs in the library and so on?

Copy link
Member

@jghauser jghauser left a comment

Choose a reason for hiding this comment

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

This is definitely useful. I only have some general remarks:

  • the author check would be more useful if it weren't triggered when an editor is defined instead (I get around 100 files simply because they are collections with just an editor defined)
  • maybe the message could be "keys X missing" just to be more explicit
  • might be cool to offer going through the list of errors one by one and offering to e.g. ignore/edit info.yaml
  • might also be cool to somehow do a check automatically when adding new entries to the library (not quite sure how or at what stage though)
  • i think checking for non-unique refs would be useful!

@alejandrogallo
Copy link
Member Author

  • I added a duplicated-keys check, which does the duplication testing, I'm still not really
    convinced about the erogonomicity of the whole, but I think it's a good start.
  • About the going one by one, I agree, maybe I can add an -i, --interactive flag
    to then do an interactive action in every relevant case.
  • About the checking automatically everytime one adds, I kind of agree, however I'm also not
    sure how.
  • About the editor, yes, you're right, but I don't know how to do it generally,
    I could create a test that does this ad-hoc, and maybe write it in the documentation
    as an example of how people could write this in theyr config.py file to register a check.

thank you for the suggestions!

@jghauser
Copy link
Member

jghauser commented Dec 4, 2022

Looking at #412 made me realise it might be useful for papis-doctor to also by default check whether entries have a (valid) type. After all, this is required for exporting to bibtex, which is probably something that many/most want to work.

@alejandrogallo
Copy link
Member Author

that's a great idea, I'll implement the check

@jghauser
Copy link
Member

jghauser commented Dec 7, 2022

I've come across some things that could benefit from being detected here:

  • all caps titles and authors (or really in all/most string values?)
  • "&amp" instead of "&"

@alejandrogallo
Copy link
Member Author

  • I can create an all-caps check where the user gives the keys where
    she wants to detect the caps. However it's difficult to have a fix
    action in general for any kind of string, because you want the
    Author Names capitalized like in Bin Mahmoud, Hasan and so on.
  • A check that checks if there are html codes in the titles, like
    & and so on. In this case the auto fix would be to replace the
    html code by their unicode equivalents.

@alejandrogallo
Copy link
Member Author

This last commit introduces initial support for webapp

image

papis/commands/serve.py Fixed Show fixed Hide fixed
import os
from typing import Optional, List, Dict, Any

import yaml # lgtm [py/import-and-import-from]

Check notice

Code scanning / CodeQL

Module is imported with 'import' and 'import from'

Module 'yaml' is imported with both 'import' and 'import from'.
@alejandrogallo
Copy link
Member Author

I'll merge this behemoth because I want to start with the papis_id, there are still a couple of things to do here &ut everything is usable and I will clean them up for v0.13

@alejandrogallo alejandrogallo merged commit 11b7f9b into papis:master Dec 20, 2022
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.

None yet

3 participants