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

More linting #355

Merged
merged 18 commits into from
Nov 27, 2023
Merged

More linting #355

merged 18 commits into from
Nov 27, 2023

Conversation

ssssarah
Copy link
Contributor

@ssssarah ssssarah commented Oct 30, 2023

  • rm import from kgforge.commons.archetypes and kgforge.core and target specific files to avoid cyclic dependencies
  • When catching an exception and re-raising a different one, raise from the one that was caught to keep the stack trace and facilitate debugging
  • not_supported to return exception to be raised
  • not_supported raised in specialisations, not core. In core, methods defined as abstract. Motivation: when implementing one of the archetypes, it's simpler to extend it and to have your IDE generate all abstract methods than it is to have them "implemented for you as not implemented" and having to go to the archetype definition to retrieve the method signatures to override. Moreover, it's better to have a complete visual of unimplemented functionality in one's specialisation, rather than to have to go back to the archetype to see methods that have yet to be implemented
  • rm redefinition of constructor in subclasses if no additional logic is added
  • rm raise Exception, raise more specific exceptions
  • Raising linting threshold to 9.0

@codecov-commenter
Copy link

codecov-commenter commented Oct 30, 2023

Codecov Report

Attention: 159 lines in your changes are missing coverage. Please review.

Comparison is base (4d2580e) 74.42% compared to head (16e50d1) 74.46%.

Files Patch % Lines
kgforge/specializations/stores/demo_store.py 30.76% 36 Missing ⚠️
kgforge/core/archetypes/model.py 51.61% 15 Missing ⚠️
kgforge/core/archetypes/resolver.py 62.96% 10 Missing ⚠️
kgforge/core/archetypes/store.py 52.38% 10 Missing ⚠️
kgforge/core/conversions/rdf.py 33.33% 8 Missing ⚠️
kgforge/specializations/models/demo_model.py 68.18% 7 Missing ⚠️
kgforge/core/archetypes/read_only_store.py 50.00% 6 Missing ⚠️
kgforge/specializations/models/rdf_model.py 66.66% 6 Missing ⚠️
kgforge/specializations/stores/bluebrain_nexus.py 60.00% 6 Missing ⚠️
kgforge/specializations/stores/sparql_store.py 45.45% 6 Missing ⚠️
... and 20 more
Additional details and impacted files
@@            Coverage Diff             @@
##           master     #355      +/-   ##
==========================================
+ Coverage   74.42%   74.46%   +0.03%     
==========================================
  Files         100      100              
  Lines        6213     6293      +80     
==========================================
+ Hits         4624     4686      +62     
- Misses       1589     1607      +18     
Flag Coverage Δ
unittests 74.46% <56.43%> (+0.03%) ⬆️

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.

@ssssarah ssssarah changed the title rm import from archetypes and core and target specific files to avoid… More linting Oct 30, 2023
@ssssarah ssssarah requested review from MFSY and crisely09 October 30, 2023 19:57
@ssssarah ssssarah marked this pull request as ready for review October 30, 2023 19:57
tox.ini Show resolved Hide resolved
@crisely09
Copy link
Contributor

These are small but wide changes, can we merge this after the archetypes change?

@ssssarah
Copy link
Contributor Author

These are small but wide changes, can we merge this after the archetypes change?

Yeah of course, archetypes > everything else

@ssssarah ssssarah requested a review from crisely09 November 27, 2023 13:04
tox.ini Show resolved Hide resolved
Copy link
Contributor

@crisely09 crisely09 left a comment

Choose a reason for hiding this comment

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

Good with me.

@ssssarah ssssarah merged commit bcc6b33 into master Nov 27, 2023
1 check passed
@ssssarah ssssarah deleted the more_linting branch November 27, 2023 14:05
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