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 external links to files (if available) and display in context menu #37

Merged
merged 12 commits into from
Mar 14, 2023

Conversation

dereklu888
Copy link
Contributor

Not finished, just adding a pr to keep track of progress.

Currently, the code adds an "external_urls" property that contains a list of external URLs to file metadata, but this has only been tested on a small dataset (templateflow's tpl-Fischer344).

Still need to do more testing and implement it on the webui side.

@dereklu888
Copy link
Contributor Author

dereklu888 commented Sep 21, 2021

Ok, did some more testing using tpl-Fischer344, the web UI portion seems to be working. However, I haven't done a full test, as in one smooth process from "datalad create-sibling" then doing "datalad push" yet. I've just been copying over the updated metadata file from my updated "datalad ls" over -- I think I'll need to integrate my updated "datalad ls" into "datalad push" somehow for that full test, I'll look into it!

(The attached photos show what it looks like when there's only 1 external URL and multiple external URLs. If there's multiple, it displays a max of 5 and lists each with their hostname)

image
image

@dereklu888
Copy link
Contributor Author

Sorry for the delay, but I've been able to test a full create-sibling to push process using the latest datalad version (15.01), and got the external_urls going as shown above.

Co-authored-by: Yaroslav Halchenko <debian@onerussian.com>
@mih
Copy link
Member

mih commented Feb 24, 2022

@yarikoptic what are your plans here? Put in draft mode?

@mih
Copy link
Member

mih commented Mar 2, 2022

I am putting this PR in draft mode.

@mih mih marked this pull request as draft March 2, 2022 11:35
@yarikoptic
Copy link
Member

thanks @mih . I think changes are LGTM, I might try to find some time to test myself and deploy and merge then.

@yarikoptic yarikoptic added the minor Increment the minor version when merged label Apr 29, 2022
@yarikoptic
Copy link
Member

after #51 is merged, I will merge master into this and ensure it breaks nothing and merge then

@yarikoptic yarikoptic marked this pull request as ready for review September 30, 2022 12:18
import datalad.api as dl

# remove the below 2 lines when NEEDS_CONTENT is already made false
import datalad.metadata.extractors.datalad_core as dc
Copy link
Member

Choose a reason for hiding this comment

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

Actually I will adjust this part since we already moved that etc

* origin/master: (79 commits)
  Update CHANGELOG.md [skip ci]
  After moving with_testrepos here, also move its tests
  Adjust import of with_testrepo to local module
  Absorb with_testrepos and studyforrest_mockup functionality
  Update CHANGELOG.md [skip ci]
  Add deprecated metadata config procedure from datalad core
  Update CHANGELOG.md [skip ci]
  List metadata commands in README
  Adjust imports for metadata move
  BF: Fix annotate path imports
  Update CHANGELOG.md [skip ci]
  use splitlines to improve behavior in multi-OS env
  reformat code to take advantage of paranthesis
  adapt to new changes in datalad master
  Update CHANGELOG.md [skip ci]
  ENH: tune up testing infrastructure
  BF(TST): make it ok to have no "notneeded" in publish test
  Configure Dependabot to update GitHub Actions action versions
  Update GitHub Actions action versions
  Update CHANGELOG.md [skip ci]
  ...
…ted too by now

=== Do not change lines below ===
{
 "chain": [],
 "cmd": "git-sedi 'import datalad.metadata' 'import datalad_deprecated.metadata'",
 "exit": 0,
 "extra_inputs": [],
 "inputs": [],
 "outputs": [],
 "pwd": "."
}
^^^ Do not change lines above ^^^
@yarikoptic
Copy link
Member

eh, crippled fs fail seems related to the changes in PR so will need to get closer look

FAILED ../tests/test_ls_webui.py::test_fs_traverse - TypeError: argument of type 'NoneType' is not iterable
FAILED ../tests/test_ls_webui.py::test_ls_json - TypeError: argument of type 'NoneType' is not iterable

@yarikoptic
Copy link
Member

Appveyor Mac fail is unrelated IMHO

Warning: Your Xcode (12.3) is outdated.
Please update to Xcode 12.4 (or delete it).
Xcode can be updated from the App Store.

@yarikoptic yarikoptic merged commit 400d2c4 into datalad:master Mar 14, 2023
@github-actions
Copy link

🚀 PR was released in 0.3.0 🚀

@github-actions github-actions bot added the released This issue/pull request has been released. label Mar 22, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
minor Increment the minor version when merged released This issue/pull request has been released.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants