JSONStore: file_writable -> read_only

[rkingsbury]

This PR adds a read_only kwarg to JSONStore to replace file_writable.

Contributor Checklist

• I have broken down my PR scope into the following TODO tasks
  • add read_only
  • add backwards compatibility logic to continue supporting file_writable
• I have run the tests locally and they passed.
• I have added tests, or extended existing tests, to cover any new features or bugs fixed in this PR

[codecov]

Codecov Report

Merging #625 (a5e72f3) into main (e1271e0) will increase coverage by 0.02%.
The diff coverage is 100.00%.

@@            Coverage Diff             @@
##             main     #625      +/-   ##
==========================================
+ Coverage   89.22%   89.24%   +0.02%     
==========================================
  Files          40       40              
  Lines        2811     2817       +6     
==========================================
+ Hits         2508     2514       +6     
  Misses        303      303              

Impacted Files	Coverage Δ
src/maggma/stores/mongolike.py	87.87% <100.00%> (+0.20%)	⬆️

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update e1271e0...a5e72f3. Read the comment docs.

[munrojm]

I think we should maybe only keep read_only as an explicit argument that is passed. We can then absorb any mention of file_writable into **kwargs. Do you have an opinion on that?

[rkingsbury]

I think we should maybe only keep read_only as an explicit argument that is passed. We can then absorb any mention of file_writable into **kwargs. Do you have an opinion on that?

Just to make sure I understand what you're saying, do you mean, e.g.

class JSONStore(MemoryStore):
    """
    A Store for access to a single or multiple JSON files
    """

    def __init__(
        self,
        paths: Union[str, List[str]],
        read_only: bool,
        **kwargs,
    ):

in other words, the user always has to specify read_only and file_writable is handled internally, with no mention in the docstring?

[munrojm]

Yeah, exactly.

[rkingsbury]

OK, will this cause compatibility problems though? Because if we're trying to support legacy code that says, e.g. js=JSONStore(<filename>) that would start to fail without the newly-required read_only arg.

[munrojm]

Oh, sorry for not being clear. It can be optional, I just wanted to suggest changing how file_writable is handled so it isn't explicitly in the function signature anymore.