Changes and updates for metadata search

[jgrethe]

This is done in via a fork of the main branch and with a run of formatters. Discarded formatting changes for files that were not originally changed for the metadata services.

[jgrethe]

@athril I think this looks good - all checks passed.

[athril]

1. I am wondering if changing docs/examples_metadata.py into a jupyter notebook tutorial won't make more sense. That way we could have a module specific tutorial.
2. A couple of remarks regarding the code.

[jgrethe]

@athril Went through the comments. Some things can be included in future releases. We can setup tickets for those things (e.g. Jupyter notebook, etc). But that should not hold up this version.

New feature request (Jupyter notebook) added as new ticket. Can be included in future update.

[jgrethe]

@athril Went through your review. Replied to your comments and also added ticket for future features (e.g. Jupyter notebook). Most of comments were about the examples file and that should stay as is as per the comments. Also had some comments about setting URL (also answered in comments) and about the getURL function (works now - so any updates/improvements can be added in future updates).

[hsorby]

The changes requested by @athril and myself should be made now. This PR is incomplete without them.

[jgrethe]

@athril @hsorby Python notebook added based on README.io tutorial so that they match. Also made suggested change to the metadata client and placed URIs into init. Removed the example python file as this is now in a notebook - so all comments on the file are not applicable anymore

[jgrethe]

@hsorby All changes suggested from last review have been committed

[hsorby]

Some more minor edits. Plus there are a few formatting warnings reported by reviewdog that need to be addressed.

[athril]

Thank you for your updates @jgrethe !

Could you possibly verify if the code passes tests and has 100% code coverage?

You could check it by running from console:

PYTHONPATH=src                                                                                                                                                                                                    
pytest --cov=./src tests/      

I ran it on linux and it seems to me that merging this pull request would break our code base (fail a couple of test_zinc.py tests). The reason we haven't noticed it is because CI.yml was not run on subsequent commits to the pull request. This will hopefully be fixed in #37.

---------- coverage: platform linux, python 3.10.12-final-0 ----------
Name                                     Stmts   Miss  Cover                                                          
------------------------------------------------------------                                                          
src/sparc/__init__.py                        3      0   100%                                                          
src/sparc/client/__init__.py                 2      0   100%                                                          
src/sparc/client/client.py                  47      0   100%                                                          
src/sparc/client/services/__init__.py       13      0   100%                                                          
src/sparc/client/services/_default.py       15      0   100%                                                          
src/sparc/client/services/metadata.py      105     74    30%                                                          
src/sparc/client/services/o2sparc.py        97      0   100%                                                          
src/sparc/client/services/pennsieve.py      67      0   100%                                                          
src/sparc/client/zinchelper.py             112     28    75%                                                          
------------------------------------------------------------                                                          
TOTAL                                      461    102    78%       


=============================================== short test summary info ================================================
FAILED tests/test_zinc.py::test_export_scaffold_into_vtk_format - TypeError: 'NoneType' object is not subscriptable 
FAILED tests/test_zinc.py::test_export_scaffold_into_stl_format - TypeError: 'NoneType' object is not subscriptable   
FAILED tests/test_zinc.py::test_export_scaffold_into_vtk_format_with_default_output_location - TypeError: 'NoneType' object is not subscriptable 
FAILED tests/test_zinc.py::test_export_mbf_to_vtk - TypeError: 'NoneType' object is not subscriptable                 
FAILED tests/test_zinc.py::test_export_mbf_to_vtk_with_default_output_name - TypeError: 'NoneType' object is not subscriptable  
5 failed, 41 passed, 14 warnings in 6.77s  

[athril]

It seems that tests/test_zinc.py issues may not be related to your pull request.
Still there seem to be no tests included in this pull request.

[jgrethe]

@athril We have local tests we run with an API key (not to be shared via Github). Where can such a private key for the configuration file be placed (where it is not made public)?

[athril]

@athril We have local tests we run with an API key (not to be shared via Github). Where can such a private key for the configuration file be placed (where it is not made public)?

Is there a chance to cover the code on Github, @jgrethe? I would rather avoid API calls if not absolutely necessary.

You could potentially mock some of the functions as @pytest.fixtures, e.g.:
https://github.com/nih-sparc/sparc.client/blob/87c7056e7bd5870ed9e758b3038804f1c6afae49/tests/conftest.py#L23C1-L37C31

Basically you define what should be returned by a mocked function and then call them directly from the test, e.g.:

sparc.client/tests/test_pennsieve.py

Lines 40 to 47 in 87c7056

	def test_get_profile(mocker, mock_pennsieve, mock_user):
	expected = "user"
	mocker.patch("pennsieve2.Pennsieve.get_user", mock_user.get_user)
	mock_user.set_user("user")
	p = PennsieveService(connect=False)
	actual = p.get_profile()
	assert actual == expected

If there is no workaround, we could create an access token in the repository secrets, but this would be the last resort.

[codecov]

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (70c6b11) 99.71% compared to head (fe1dad9) 100.00%.
Report is 31 commits behind head on main.

Additional details and impacted files

@@             Coverage Diff             @@
##             main       #29      +/-   ##
===========================================
+ Coverage   99.71%   100.00%   +0.28%     
===========================================
  Files           8         9       +1     
  Lines         356       446      +90     
===========================================
+ Hits          355       446      +91     
+ Misses          1         0       -1     

Flag	Coverage Δ
unittests	100.00% <100.00%> (?)

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[jgrethe]

@athril I checked in the pytests for metadata. The URL handling was also streamlined a bit and an additional check was added to the list and search methods if the user changes the default URL

[jgrethe]

Hi @athril - any updates on the updates above?

[athril]

Looks good to me