signac icon indicating copy to clipboard operation
signac copied to clipboard

Implement feature to encode and retrieve signac elements by URI.

Open csadorf opened this issue 6 years ago • 18 comments
trafficstars

Description

This feature implements the ability to encode certain signac elements as a URI which can then be used to retrieve said element with the signac.open() function.

Minimal example:

>>> uri = project.find_jobs({'foo' 0, 'doc.bar': True}).to_uri()
>>> print(uri)
signac://localhost/Users/csadorf/my_project/api/v1/find?sp.foo=0&doc.bar=True
>>> signac.open(uri)
signac.contrib.project.JobsCursor({'project': 'project', 'filter': '{'sp.foo': 0, 'doc.bar': 'True'}'})

Related to issue #96.

Motivation and Context

This change is motivated by the desire to reliably and unambiguously encode signac elements as unique text-strings. It is much easier to copy & paste such a URI and store it somewhere, instead of awkwardly JSON-encoded filter for example.

Types of Changes

  • [ ] Documentation update
  • [ ] Bug fix
  • [x] New feature
  • [ ] Breaking change1

1The change breaks (or has the potential to break) existing functionality.

Checklist:

If necessary:

  • [ ] I have updated the API documentation as part of the package doc-strings.
  • [ ] I have created a separate pull request to update the framework documentation on signac-docs and linked it here.
  • [ ] I have updated the changelog.

csadorf avatar May 19 '19 02:05 csadorf

Codecov Report

Merging #189 into feature/integrated-queries will decrease coverage by 1.24%. The diff coverage is 37.17%.

Impacted file tree graph

@@                      Coverage Diff                       @@
##           feature/integrated-queries     #189      +/-   ##
==============================================================
- Coverage                       65.03%   63.79%   -1.25%     
==============================================================
  Files                              37       38       +1     
  Lines                            5554     5626      +72     
==============================================================
- Hits                             3612     3589      -23     
- Misses                           1942     2037      +95
Impacted Files Coverage Δ
signac/__init__.py 100% <100%> (ø) :arrow_up:
signac/contrib/filterparse.py 67.59% <26.08%> (-10.06%) :arrow_down:
signac/uri.py 30.43% <30.43%> (ø)
signac/contrib/project.py 87.29% <48.27%> (-2.89%) :arrow_down:
signac/contrib/job.py 85.82% <50%> (-4.65%) :arrow_down:
signac/core/json.py 81.57% <0%> (-13.16%) :arrow_down:
signac/common/errors.py 72.22% <0%> (-11.12%) :arrow_down:
signac/contrib/indexing.py 73.77% <0%> (-1.48%) :arrow_down:
signac/core/jsondict.py 93.86% <0%> (-1.23%) :arrow_down:
... and 11 more

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 8487e60...e67ddd4. Read the comment docs.

codecov[bot] avatar May 19 '19 14:05 codecov[bot]

Codecov Report

Merging #189 into feature/integrated-queries will increase coverage by 0.17%. The diff coverage is 80.6%.

Impacted file tree graph

@@                      Coverage Diff                       @@
##           feature/integrated-queries     #189      +/-   ##
==============================================================
+ Coverage                        65.1%   65.28%   +0.17%     
==============================================================
  Files                              37       38       +1     
  Lines                            5574     5657      +83     
==============================================================
+ Hits                             3629     3693      +64     
- Misses                           1945     1964      +19
Impacted Files Coverage Δ
signac/__main__.py 0% <0%> (ø) :arrow_up:
signac/contrib/import_export.py 84.93% <100%> (ø) :arrow_up:
signac/contrib/schema.py 89.16% <100%> (ø) :arrow_up:
signac/contrib/linked_view.py 85.71% <100%> (ø) :arrow_up:
signac/__init__.py 100% <100%> (ø) :arrow_up:
signac/contrib/job.py 90.15% <50%> (-0.32%) :arrow_down:
signac/uri.py 78.26% <78.26%> (ø)
signac/contrib/project.py 89.91% <84.09%> (-0.18%) :arrow_down:
signac/contrib/filterparse.py 84.8% <89.87%> (+3.35%) :arrow_up:
... and 3 more

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 99a5847...8750e88. Read the comment docs.

codecov[bot] avatar May 19 '19 14:05 codecov[bot]

@bdice Is this something that we can integrate with dashboard?

csadorf avatar May 19 '19 22:05 csadorf

@bdice Is this something that we can integrate with dashboard?

Probably, but I'm not sure if it even makes sense if the URIs require absolute paths. Is there a "relative path" variant of this planned? Like a Project.open(...)?

bdice avatar May 29 '19 02:05 bdice

@bdice Is this something that we can integrate with dashboard?

Probably, but I'm not sure if it even makes sense if the URIs require absolute paths. Is there a "relative path" variant of this planned? Like a Project.open(...)?

Yes, Project.open(...) is already implemented as part of this PR.

csadorf avatar Jun 07 '19 14:06 csadorf

@csadorf Can you help me understand what I should do as the assignee on this PR? It's in a "draft" form, so did you still want me to find a second reviewer? It also appears that this PR is out of sync with its target branch -- there is some stuff about signac command history showing in the diff that seems unrelated to the title of this PR.

bdice avatar Jul 07 '19 04:07 bdice

@csadorf Can you help me understand what I should do as the assignee on this PR? It's in a "draft" form, so did you still want me to find a second reviewer? It also appears that this PR is out of sync with its target branch -- there is some stuff about signac command history showing in the diff that seems unrelated to the title of this PR.

No, I'd say at the draft stage you should just make sure that there is some kind of movement with the PR and help the creator (in this case me), move the PR to the point that is "ready for review". Or close it if it completely stalls out, i.e., the creator doesn't respond anymore and you can't find anyone to take over.

csadorf avatar Jul 07 '19 14:07 csadorf

Or close it if it completely stalls out [...]

@csadorf What are appropriate next steps for this draft PR? I've dropped it from the v1.3 release scheduled for next week. Should it be closed for now?

bdice avatar Dec 06 '19 20:12 bdice

Please add to 1.4 milestone and leave it open, thx.

csadorf avatar Dec 06 '19 20:12 csadorf

I think this will be a really great feature, I've added it to the milestone for v1.5

mikemhenry avatar Apr 08 '20 20:04 mikemhenry

We should revisit this PR after ~#188~ #332 is merged, it is pretty close to complete. As discussed with @csadorf, it might be beneficial to get this reviewed by someone more experienced with working with RESTful APIs (i.e. a web developer) to make sure that the URIs we're proposing make sense. @bdice @mikemhenry in any case we should plan to discuss plans for this PR at the dev meeting on Monday.

vyasr avatar Apr 25 '20 22:04 vyasr

Brief note: We should probably directly support IRIs as a superset of URIs.

csadorf avatar Apr 28 '20 13:04 csadorf

@vyasr This is no longer blocked as #332 is merged, correct?

csadorf avatar Mar 25 '21 15:03 csadorf

Correct, this is no longer blocked. However, at this point I think we should plan this as a 2.1 feature so that we can focus our immediate efforts on finishing up the changes required for 2.0. I think this is the next major feature on the docket, though.

vyasr avatar Mar 25 '21 21:03 vyasr

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

stale[bot] avatar Jun 02 '21 16:06 stale[bot]

@vyasr and @csadorf, is this still more appropriate as a 2.1 feature, or can it go into 2.0?

tcmoore3 avatar Aug 10 '21 13:08 tcmoore3

I don't know how much more work needs to be done here. I'd leave it up to the judgement of @vyasr .

csadorf avatar Aug 10 '21 13:08 csadorf

Sorry for not getting back to this any sooner. I think this is still more appropriate for 2.1. We have enough to get done to get us to a clean state for 2.0 that we'd be better served getting our ducks in line for that and then coming back to this in 2.1.

vyasr avatar Sep 29 '21 02:09 vyasr