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

Maint: ebrains datasets, private some properties and methods #444

Merged
merged 2 commits into from
Sep 8, 2023

Conversation

AhmetNSimsek
Copy link
Collaborator

This was a result of a conversation with @dickscheid on 29.08.2023. detail property and parse_person() should be private as the users have no direct use for it.

@xgui3783, would you mind writing a doc string for _parse_person?

@codecov-commenter
Copy link

Codecov Report

Merging #444 (6bdca8e) into main (5a0e6b3) will increase coverage by 0.70%.
Report is 92 commits behind head on main.
The diff coverage is 27.46%.

❗ Your organization is not using the GitHub App Integration. As a result you may experience degraded service beginning May 15th. Please install the Github App Integration for your organization. Read more.

@@            Coverage Diff             @@
##             main     #444      +/-   ##
==========================================
+ Coverage   36.81%   37.52%   +0.70%     
==========================================
  Files          61       61              
  Lines        5421     5538     +117     
==========================================
+ Hits         1996     2078      +82     
- Misses       3425     3460      +35     
Files Changed Coverage Δ
siibra/configuration/configuration.py 42.35% <ø> (ø)
siibra/configuration/factory.py 22.22% <0.00%> (+0.24%) ⬆️
siibra/core/atlas.py 85.71% <ø> (+1.23%) ⬆️
...a/features/connectivity/functional_connectivity.py 55.55% <0.00%> (ø)
siibra/livequeries/ebrains.py 25.35% <0.00%> (+0.35%) ⬆️
siibra/locations/boundingbox.py 19.88% <ø> (ø)
siibra/volumes/neuroglancer.py 21.42% <0.00%> (ø)
siibra/volumes/volume.py 50.29% <0.00%> (ø)
siibra/features/tabular/cortical_profile.py 26.43% <3.70%> (-4.23%) ⬇️
siibra/features/tabular/tabular.py 23.68% <3.70%> (-6.32%) ⬇️
... and 23 more

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

@xgui3783
Copy link
Member

IMO, this would be a breaking change, since we turn public attr to private attr

Why turn them private anyway?

@AhmetNSimsek
Copy link
Collaborator Author

I would normally agree but, in this case, we could not think of a reason for them to be exposed to the user. That is, it is not the same as hiding Feature.anchor which is frequently used. Does this impact siibra-api?

@AhmetNSimsek AhmetNSimsek changed the base branch from main to v0.5_dev September 8, 2023 09:02
@AhmetNSimsek AhmetNSimsek merged commit c82e7a9 into v0.5_dev Sep 8, 2023
@AhmetNSimsek AhmetNSimsek deleted the maint_ebrainsdatasets branch September 8, 2023 10:08
AhmetNSimsek added a commit that referenced this pull request Nov 8, 2023
Maint: ebrains datasets, detail property should be private (#444)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants