Adds {journal} field to file options and tests it

[boyanpenkov]

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]

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]

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

[boyanpenkov]

@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]

@perrette -- just touching base here...

[boyanpenkov]

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

[boyanpenkov]

@perrette -- hello, just reaching out to see what your opinion here is...

Thanks kindly!

[perrette]

Hi @boyanpenkov , I am back doing some minor checks on papers, I'll look into it now. I merged perfect-undo into master, BTW. Can that PR be redirected towards master somehow?

[boyanpenkov]

Oh, super -- thanks; do let me know. I'd be glad for an opinion here...

[perrette]

Ok so I did merge the PR into master. The only thing I changed was the tests: you had commented a bunch of things that were actually useful (testing the full renamed path). The issue I think you were trying to address (an error when checking the paths) came from an unwanted side-effect of modifying the file template that also inadvertedly modify the global default template, and so your journal modification was persistent throughout the rest of the code. I simply added a copy.deepcopy for the keyformat and nameformat fields of the config class and that fixed it.

[boyanpenkov]

Oh, this is fantastic -- super; I'll take a look here (please bear with me -- very tied up with the end of my thesis here...).

Yes, if memory served the root cause of the observation was that tests explicitly relied on the local settings file. I'll take a look here when I come up for air. (and yes, sorry about temporarily commenting out things that were useful that should ultimately be run, in my working copy).