[MISC] Remove depencency of seqan3 io #44

Irallia · 2022-01-05T13:22:34Z

Resolves #42

To avoid using seqan3::io we changed the usage of the seqan3::input_file_validator and seqan3::output_file_validator, you now have to give the formats explicitly:
sharg::input_file_validator validator{std::vector{std::string{"exe"}, std::string{"fasta"}}}; and sharg::output_file_validator validator{sharg::output_file_open_options::create_new, std::vector{std::string{"exe"}, std::string{"fasta"}}};.

I also created a new issue seqan/seqan3#2927 for accessing valid formats from file.

vercel · 2022-01-05T13:22:38Z

This pull request is being automatically deployed with Vercel (learn more).
To see the status of your deployment, click below or on the icon next to each commit.

🔍 Inspect: https://vercel.com/seqan/sharg-parser/DxuTvYtkqc7W28An5H3BZvMrcwwV
✅ Preview: https://sharg-parser-git-fork-irallia-misc-removedepencenc-429826-seqan.vercel.app

include/sharg/validators.hpp

Irallia · 2022-01-05T14:39:59Z

test/snippet/validators_input_file_ext_from_file.cpp

-    // Give the seqan3 file type as a template argument to get all valid extensions for this file.
-    sharg::input_file_validator<seqan3::sequence_file_input<>> validator3{};
+    // Give the sharg file type as a template argument to get all valid extensions for this file.
+    sharg::input_file_validator validator3{seqan3::sequence_file_input<>::valid_formats};


I missed testing the snippets and unfortunately I get an error for the new notation:

Consolidate compiler generated dependencies of target validators_3_snippet [ 40%] Building CXX object CMakeFiles/validators_3_snippet.dir/validators_3.cpp.o /Users/lydia/Repos/sharg-parser/test/snippet/validators_input_file_ext_from_file.cpp: In function 'int main()': /Users/lydia/Repos/sharg-parser/test/snippet/validators_input_file_ext_from_file.cpp:16:88: error: expected primary-expression before '}' token 16 | sharg::input_file_validator validator3{seqan3::sequence_file_input<>::valid_formats}; | ^ /Users/lydia/Repos/sharg-parser/test/snippet/validators_input_file_ext_from_file.cpp:16:88: error: no matching function for call to 'sharg::input_file_validator::input_file_validator(<brace-enclosed initializer list>)'

In addition, I need with the spelling then further the include: <seqan3/io/sequence_file/input.hpp>, or was that clear?

ops. I was to quick. It seems like you access the extensions like this:
seqan3::detail::valid_file_extensions<typename seqan3::sequence_file_input<>::valid_formats>()

I did not know that there is no nice way yet to access the extensions. I don't remember why the file class itself does not have a member for this. @eseiler Do you remember? There is nothing in the core meeting notes. I feel like someone had a say in this but I just don't why the file shouldn't have a member with the allowed extensions like
seqan3::sequence_file_input<>::file_extensions

As an alternative we could add a helper function
seqan3::get_file_extensions<seqan3::sequence_file_input<>>() that just calls the detail function, or make the detail function public.

Alternative

We could also remove this example for now. As @Irallia mentioned, the snippet will this way need the seqan3 include anyway. Which we don't want. So maybe this has to be deleted anyway or moved to IO snippets. Then this can be tackled as a follow up in seqan3.

The problem with the helper is that it is detail.
I'm not sure why it isn't a member, maybe we never discussed it.

I only remember that we discussed compression extensions.
In the end, I implemented compression extension like this https://github.com/seqan/raptor/blob/45de7f8d3ce5f21a0f5bd9ceb35173acbfa5867d/include/raptor/argument_parsing/validators.hpp#L159-L184

I would remove the IO part in this PR from the argument parser and make this a seqan3 issue about how to access the extensions.

I created an issue for this:
seqan/seqan3#2927
Feel free to add anything or rewrite it.

smehringer · 2022-01-06T12:16:57Z

CHANGELOG.md

+
+#### I/O
+
+To avoid using seqan3::io we changed the usage of the `seqan3::input_file_validator` from


Suggested change

To avoid using seqan3::io we changed the usage of the `seqan3::input_file_validator` from

* To avoid using seqan3::io we changed the usage of the `seqan3::input_file_validator` from

Irallia · 2022-01-10T11:26:15Z

We have still have one io include in include/sharg/auxiliary.hpp:

#include <seqan3/io/stream/concept.hpp>

... seqan3::input_stream_over ...

EDIT: Will be done by #41

codecov · 2022-01-10T12:59:47Z

Codecov Report

Merging #44 (915bddf) into master (f1104c8) will decrease coverage by 0.02%.
The diff coverage is 100.00%.

❗ Current head 915bddf differs from pull request most recent head ec6060f. Consider uploading reports for the commit ec6060f to get more accurate results

@@            Coverage Diff             @@
##           master      #44      +/-   ##
==========================================
- Coverage   95.23%   95.21%   -0.03%     
==========================================
  Files          13       13              
  Lines         923      919       -4     
==========================================
- Hits          879      875       -4     
  Misses         44       44

Impacted Files	Coverage Δ
include/sharg/argument_parser.hpp	`97.53% <ø> (ø)`
include/sharg/validators.hpp	`100.00% <100.00%> (ø)`

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update f1104c8...ec6060f. Read the comment docs.

smehringer

Almost done.

smehringer · 2022-01-12T07:58:47Z

CHANGELOG.md

+* To avoid using seqan3::io we changed the usage of the `seqan3::input_file_validator` and
+`seqan3::output_file_validator`, you now have to give the formats explicitly:
+`sharg::input_file_validator validator{std::vector{std::string{"exe"}, std::string{"fasta"}}};` and
+`sharg::output_file_validator validator{sharg::output_file_open_options::create_new, std::vector{std::string{"exe"}, std::string{"fasta"}}};`.


Suggested change

* To avoid using seqan3::io we changed the usage of the `seqan3::input_file_validator` and

`seqan3::output_file_validator`, you now have to give the formats explicitly:

`sharg::input_file_validator validator{std::vector{std::string{"exe"}, std::string{"fasta"}}};` and

`sharg::output_file_validator validator{sharg::output_file_open_options::create_new, std::vector{std::string{"exe"}, std::string{"fasta"}}};`.

* In order to avoid using the seqan3 I/O module, you now have to give a list of file extensions explicitly to `sharg::input_file_validator` and

`sharg::output_file_validator`: For example `sharg::input_file_validator validator{std::vector<std::string>{{"exe"}, {"fasta"}}};`. Please follow https://github.com/seqan/seqan3/issues/2927 to see how the list of file extensions can be extracted from seqan3 files.

smehringer · 2022-01-12T07:59:41Z

include/sharg/validators.hpp

@@ -12,12 +12,12 @@

 #pragma once

+#include <seqan3/std/algorithm>


we can fix that in a separate PR. I will make a follow up issue

fixed here #47

include/sharg/validators.hpp

smehringer · 2022-01-12T08:12:33Z

include/sharg/validators.hpp

    /*!\brief The default extensions of `file_t`.
     * \returns A list of default extensions for `file_t`, will be empty if `file_t` is `void`.
-     *
-     * \details
-     *
-     * If `file_t` does name a valid seqan3 file type that contains a static member `valid_formats` returns the
-     * extensions of that `file_t` type. Otherwise returns an empty list.
     */
    static std::vector<std::string> default_extensions()
    {
-        if constexpr (!std::same_as<file_t, void>)
-            return seqan3::detail::valid_file_extensions<typename file_t::valid_formats>();
        return {};
    }


I think you can remove the entire function as it makes no sense anymore to provide it? This should also be mentioned in the changelog then.

But I had to add another function, for not given extensions.

smehringer

ONly a single thing!

smehringer · 2022-01-14T07:02:15Z

include/sharg/validators.hpp

     */
-    explicit output_file_validator(output_file_open_options const mode,
-                                   std::vector<std::string> extensions = default_extensions())
+    explicit output_file_validator(output_file_open_options const mode, std::vector<std::string> extensions)


I think you can do this:

Suggested change

explicit output_file_validator(output_file_open_options const mode, std::vector<std::string> extensions)

explicit output_file_validator(output_file_open_options const mode, std::vector<std::string> extensions = {})

Which will construct an empty std::vector<std::string>{}. If it doesn't work,

Suggested change

explicit output_file_validator(output_file_open_options const mode, std::vector<std::string> extensions)

explicit output_file_validator(output_file_open_options const mode, std::vector<std::string> extensions = std::vector<std::string>{})

Will work for sure.

Your solution works well too but this way we only need a single constructor.

Sorry that it took so long. The first version already worked :)

Irallia · 2022-01-19T10:57:28Z

I`ll rebase after your final review.

smehringer · 2022-01-20T06:45:46Z

LGTM! When you rebase, you can also remove the seqan3/std/ranges include I think.

Signed-off-by: Lydia Buntrock <[email protected]>

Irallia · 2022-01-20T11:18:52Z

LGTM! When you rebase, you can also remove the seqan3/std/ranges include I think.

There is no 'seqan3/std/ranges', just a new 'seqan3/std/algorithm' but that still fails, if I remove it.

vercel bot temporarily deployed to Preview January 5, 2022 13:22 Inactive

Irallia changed the title ~~Misc/remove depencency of seqan3 io~~ [MISC] Remove depencency of seqan3 io Jan 5, 2022

Irallia force-pushed the MISC/Remove_depencency_of_seqan3_IO branch from bced500 to 4d386b9 Compare January 5, 2022 13:28

vercel bot temporarily deployed to Preview January 5, 2022 13:28 Inactive

Irallia self-assigned this Jan 5, 2022

Irallia force-pushed the MISC/Remove_depencency_of_seqan3_IO branch from 4d386b9 to 20cf803 Compare January 5, 2022 13:40

vercel bot temporarily deployed to Preview January 5, 2022 13:40 Inactive

smehringer reviewed Jan 5, 2022

View reviewed changes

include/sharg/validators.hpp Show resolved Hide resolved

Irallia commented Jan 5, 2022

View reviewed changes

smehringer reviewed Jan 6, 2022

View reviewed changes

Irallia force-pushed the MISC/Remove_depencency_of_seqan3_IO branch from 20cf803 to c8046b4 Compare January 10, 2022 11:25

vercel bot temporarily deployed to Preview January 10, 2022 11:25 Inactive

Irallia force-pushed the MISC/Remove_depencency_of_seqan3_IO branch from c8046b4 to 605e1ca Compare January 10, 2022 11:27

vercel bot temporarily deployed to Preview January 10, 2022 11:27 Inactive

Irallia force-pushed the MISC/Remove_depencency_of_seqan3_IO branch from 605e1ca to 3376f2a Compare January 10, 2022 12:20

vercel bot temporarily deployed to Preview January 10, 2022 12:20 Inactive

vercel bot temporarily deployed to Preview January 10, 2022 12:29 Inactive

Irallia force-pushed the MISC/Remove_depencency_of_seqan3_IO branch from a3d08e1 to 4ac919b Compare January 10, 2022 12:42

vercel bot temporarily deployed to Preview January 10, 2022 12:42 Inactive

Irallia force-pushed the MISC/Remove_depencency_of_seqan3_IO branch from 4ac919b to 99e578a Compare January 10, 2022 12:47

vercel bot temporarily deployed to Preview January 10, 2022 12:47 Inactive

Irallia force-pushed the MISC/Remove_depencency_of_seqan3_IO branch from 99e578a to 7fbaa96 Compare January 10, 2022 12:56

vercel bot temporarily deployed to Preview January 10, 2022 12:56 Inactive

This comment has been minimized.

Sign in to view

Irallia requested review from a team and MitraDarja and removed request for a team January 10, 2022 13:03

MitraDarja requested review from a team and smehringer and removed request for a team January 11, 2022 14:12

Irallia requested review from MitraDarja and a team and removed request for a team and MitraDarja January 11, 2022 16:12

smehringer requested changes Jan 12, 2022

View reviewed changes

smehringer mentioned this pull request Jan 12, 2022

[MISC] Remove dependency on ranges::move and cpp20::back_inserter. #47

Merged

vercel bot temporarily deployed to Preview January 13, 2022 12:19 Inactive

Irallia requested a review from smehringer January 13, 2022 12:19

Irallia force-pushed the MISC/Remove_depencency_of_seqan3_IO branch from a37a868 to a6e904f Compare January 13, 2022 12:25

vercel bot temporarily deployed to Preview January 13, 2022 12:25 Inactive

smehringer requested changes Jan 14, 2022

View reviewed changes

vercel bot temporarily deployed to Preview January 19, 2022 10:55 Inactive

Irallia requested a review from smehringer January 19, 2022 10:56

smehringer approved these changes Jan 20, 2022

View reviewed changes

Irallia added 4 commits January 20, 2022 12:01

[MISC] Remove dependency of seqan3::io.

305cca0

Signed-off-by: Lydia Buntrock <[email protected]>

[DOC] Add changelog entry

c1a5a75

Signed-off-by: Lydia Buntrock <[email protected]>

[MISC] Add missing headers.

17cd216

Signed-off-by: Lydia Buntrock <[email protected]>

[TEST] Add some tests.

ec6060f

Signed-off-by: Lydia Buntrock <[email protected]>

Irallia force-pushed the MISC/Remove_depencency_of_seqan3_IO branch from 915bddf to 204111a Compare January 20, 2022 11:12

vercel bot temporarily deployed to Preview January 20, 2022 11:12 Inactive

Irallia force-pushed the MISC/Remove_depencency_of_seqan3_IO branch from 204111a to ec6060f Compare January 20, 2022 11:18

vercel bot temporarily deployed to Preview January 20, 2022 11:18 Inactive

smehringer merged commit 1506aa8 into seqan:master Jan 20, 2022

smehringer mentioned this pull request Jan 24, 2022

[MISC] Remove std::ranges::copy and std::cpp20::back_inserter. #54

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[MISC] Remove depencency of seqan3 io #44

[MISC] Remove depencency of seqan3 io #44

Irallia commented Jan 5, 2022 •

edited

Loading

vercel bot commented Jan 5, 2022 •

edited

Loading

Irallia Jan 5, 2022 •

edited

Loading

smehringer Jan 5, 2022 •

edited

Loading

eseiler Jan 6, 2022

smehringer Jan 6, 2022

Irallia Jan 10, 2022

smehringer Jan 6, 2022

Irallia commented Jan 10, 2022 •

edited

Loading

codecov bot commented Jan 10, 2022 •

edited

Loading

This comment has been minimized.

smehringer left a comment

smehringer Jan 12, 2022

smehringer Jan 12, 2022

smehringer Jan 12, 2022

smehringer Jan 12, 2022

Irallia Jan 13, 2022

smehringer left a comment

smehringer Jan 14, 2022

Irallia Jan 19, 2022

Irallia commented Jan 19, 2022

smehringer commented Jan 20, 2022

Irallia commented Jan 20, 2022


		#### I/O

		To avoid using seqan3::io we changed the usage of the `seqan3::input_file_validator` from

	To avoid using seqan3::io we changed the usage of the `seqan3::input_file_validator` from
	* To avoid using seqan3::io we changed the usage of the `seqan3::input_file_validator` from

		@@ -12,12 +12,12 @@

		#pragma once

		#include <seqan3/std/algorithm>

	explicit output_file_validator(output_file_open_options const mode, std::vector<std::string> extensions)
	explicit output_file_validator(output_file_open_options const mode, std::vector<std::string> extensions = {})

[MISC] Remove depencency of seqan3 io #44

[MISC] Remove depencency of seqan3 io #44

Conversation

Irallia commented Jan 5, 2022 • edited Loading

vercel bot commented Jan 5, 2022 • edited Loading

Irallia Jan 5, 2022 • edited Loading

Choose a reason for hiding this comment

smehringer Jan 5, 2022 • edited Loading

Choose a reason for hiding this comment

Alternative

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Irallia commented Jan 10, 2022 • edited Loading

codecov bot commented Jan 10, 2022 • edited Loading

Codecov Report

This comment has been minimized.

smehringer left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

smehringer left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Irallia commented Jan 19, 2022

smehringer commented Jan 20, 2022

Irallia commented Jan 20, 2022

Irallia commented Jan 5, 2022 •

edited

Loading

vercel bot commented Jan 5, 2022 •

edited

Loading

Irallia Jan 5, 2022 •

edited

Loading

smehringer Jan 5, 2022 •

edited

Loading

Irallia commented Jan 10, 2022 •

edited

Loading

codecov bot commented Jan 10, 2022 •

edited

Loading