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

[FIX] config validation #78

Merged
merged 1 commit into from
Sep 6, 2023
Merged

Conversation

eseiler
Copy link
Member

@eseiler eseiler commented Sep 6, 2023

  • tmax shouldn't be initialised to a value other than 0, otherwise we will never set a default
  • the max number of bins for setting a default tmax was wrong
  • tmax cannot be bigger than 65472, because the next multiple of 64 would be 65536

For discussion:

  • Make tmax a size_t?
  • Initialising input_fn in the config is needed such that input_fn doesn't need to be provided when using designated intialisers (input_fn would be uninitialised otherwise, default ctor is not affected). Would be a way to make input_fn mandatory, but it doesn't really seem in the spirit of designated initialisers, and it would also hinder use cases such as first using desginated initialisers for construction, and setting input_fn afterwards (workaround would probably be .input_fn = {}).

@vercel
Copy link

vercel bot commented Sep 6, 2023

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
hibf ✅ Ready (Inspect) Visit Preview 💬 Add feedback Sep 6, 2023 2:37pm

@seqan-actions seqan-actions added lint [INTERNAL] used for linting and removed lint [INTERNAL] used for linting labels Sep 6, 2023
@codecov
Copy link

codecov bot commented Sep 6, 2023

Codecov Report

Patch coverage: 100.00% and project coverage change: +0.45% 🎉

Comparison is base (2c70085) 93.78% compared to head (27b3cdb) 94.24%.

Additional details and impacted files
@@            Coverage Diff             @@
##             main      #78      +/-   ##
==========================================
+ Coverage   93.78%   94.24%   +0.45%     
==========================================
  Files          38       38              
  Lines        1335     1337       +2     
==========================================
+ Hits         1252     1260       +8     
+ Misses         83       77       -6     
Files Changed Coverage Δ
include/hibf/config.hpp 100.00% <ø> (ø)
src/config.cpp 100.00% <100.00%> (+17.14%) ⬆️

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

@seqan-actions seqan-actions added lint [INTERNAL] used for linting and removed lint [INTERNAL] used for linting labels Sep 6, 2023
@eseiler eseiler merged commit 9066b1d into seqan:main Sep 6, 2023
26 checks passed
@eseiler eseiler deleted the fix/config_validation branch September 6, 2023 15:11
@smehringer smehringer mentioned this pull request Sep 7, 2023
25 tasks
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.

2 participants