perrette / papers

Command-line tool to manage bibliography (pdfs + bibtex)
MIT License
142 stars 22 forks source link

Adds {journal} field to file options and tests it #65

Open boyanpenkov opened 2 weeks ago

boyanpenkov commented 2 weeks ago

wip

dontmerge

closes https://github.com/perrette/papers/issues/64

I am looking at adding the {journal} option here, and thing I have tracked though the entry mechanism there. however, when I run, I see the following error still:

(python) → more_fields Repos/papers papers add ../../Literature/Stage/2013_AdvCIS_Modeling\ and\ simulation\ of\ electrostatically\ gated\ nanochannels.pdf
Traceback (most recent call last):
  File "/home/boyan/miniconda3/envs/python/bin/papers", line 8, in <module>
    sys.exit(main())
             ^^^^^^
  File "/home/boyan/Vazhno/Work/Repos/papers/papers/__main__.py", line 1195, in main
    check_install(subp, o, config) and addcmd(subp, o, config)
                                       ^^^^^^^^^^^^^^^^^^^^^^^
  File "/home/boyan/Vazhno/Work/Repos/papers/papers/__main__.py", line 534, in addcmd
    biblio.add_pdf(file, attachments=o.attachment, rename=o.rename, copy=o.copy,
  File "/home/boyan/Vazhno/Work/Repos/papers/papers/bib.py", line 435, in add_pdf
    self.insert_entry(entry, update_key=True, **kw)
  File "/home/boyan/Vazhno/Work/Repos/papers/papers/bib.py", line 288, in insert_entry
    self.insert_entry_check(entry, update_key=update_key, rename=rename, copy=copy, **checkopt)
  File "/home/boyan/Vazhno/Work/Repos/papers/papers/bib.py", line 345, in insert_entry_check
    file = merge_files([candidate, entry], relative_to=self.relative_to)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/home/boyan/Vazhno/Work/Repos/papers/papers/duplicate.py", line 290, in merge_files
    check = checksum(f) if os.path.exists(f) else None
            ^^^^^^^^^^^
  File "/home/boyan/Vazhno/Work/Repos/papers/papers/utils.py", line 109, in checksum
    return hash_bytestr_iter(file_as_blockiter(open(fname, 'rb')), hashlib.sha256())
                                               ^^^^^^^^^^^^^^^^^
IsADirectoryError: [Errno 21] Is a directory: '/home/boyan/Vazhno/Work/Literature'

@perrette -- am I missing something here? That thing is indeed a directory, which is the target for the PDFs I'd like to have.

I will end up testing this in test_add.py unless there are objections.

perrette commented 2 weeks ago

Without looking in detail, the items can be stored as name.pdf , or as name/original_name.pdf, to allow for attachments to be saved alongside (it's quite, but not entirely PDF-centric).

perrette commented 2 weeks ago

...and the checksum function can only apply to a PDF, not to a directory

boyanpenkov commented 2 weeks ago

@perrette -- Ok, ready for your perspective... with a significant caveat:

The actual feature works, and the test to check is test_add_rename_copy_journal() in test_add.py. This passes, and manually inspecting the behavior on a few files passes as well, so that's nice.

The bad thing is that the test suite runs papers directly, and so depends on the config the user has specified (which, in my case for for testing this, does have {journal} and so breaks the temp files that are written.) This breaks a lot of the undo-redo tests; all breakages there have been marked TODO and I know what they are.

What might get around this is generating an --ignore-install option, for papers to simply not read the install config and run with the consistent defaults. Alternately, are you familiar with https://docs.pytest.org/en/6.2.x/fixture.html ? Couching the tests in terms of these might be a more robust long-term solution, but is a re-work of more major sorts.

So, I think I need an opinion from the papers BDFL here. ;)

boyanpenkov commented 1 week ago

@perrette -- just touching base here...

boyanpenkov commented 1 day ago

Hello, just wanted to touch base here....