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

Update example data #58

Merged
merged 12 commits into from
Feb 27, 2024
Merged

Update example data #58

merged 12 commits into from
Feb 27, 2024

Conversation

smk78
Copy link
Contributor

@smk78 smk78 commented Jan 18, 2024

This PR updates the example data library in sasdata to match that in sasview main. This includes the supplied but not committed magnetic sans data from SasView/tutorials#13.

testdata_help.rst has also been revised accordingly.

@krzywon
Copy link
Collaborator

krzywon commented Jan 22, 2024

@smk78 - The majority of these data sets were purposely omitted during the initial move. The magnetic data directory was overlooked, but coordinate data and save states are only importable through SasView.

Coordinate data readers and data structures should be here, but are not, as of yet: SasView/sasview#2656

Save states could also live here in the future, but that would require this package to load the data.

@butlerpd
Copy link
Member

Good point. I guess we need to think hard about what package loads what data but then we want to be able to list all of them in one place for the full SasView installer I gues? But for now this is a first step I guess?

@smk78
Copy link
Contributor Author

smk78 commented Jan 25, 2024

@smk78 - The majority of these data sets were purposely omitted during the initial move. The magnetic data directory was overlooked, but coordinate data and save states are only importable through SasView.

Coordinate data readers and data structures should be here, but are not, as of yet: SasView/sasview#2656

Save states could also live here in the future, but that would require this package to load the data.

Ok. Noted. I'll take //coordinate_data and //saved_states out.

@smk78
Copy link
Contributor Author

smk78 commented Jan 25, 2024

Good point. I guess we need to think hard about what package loads what data but then we want to be able to list all of them in one place for the full SasView installer I gues? But for now this is a first step I guess?

So some thought will be needed about testdata_help.rst. This presently covers all our test/example data (ie, including those files not in or supported by sasdata).

@butlerpd
Copy link
Member

This looks like it should also be merged into 0.9 eventually so it will get bundled with 6.0?

@butlerpd
Copy link
Member

@krzywon probably needs to verify that this is now appropriate?

@krzywon krzywon changed the base branch from master to release_0.9.0 February 16, 2024 18:11
Copy link
Collaborator

@krzywon krzywon left a comment

Choose a reason for hiding this comment

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

This looks better now. I have a small issue with the help file (it still mentions SasView specific files), but I don't think that should hold this PR up

@smk78
Copy link
Contributor Author

smk78 commented Feb 23, 2024

This looks better now. I have a small issue with the help file (it still mentions SasView specific files), but I don't think that should hold this PR up

When we release what example data will actually be included:
i) only that in sasview (ie, all our example data as at present);
ii) only that in sasdata (ie, no coordinate_data or saved states);
iii) some clever synthesis of what's in sasview & sasdata so that everything is included but is dragged from the different repos?

I'm presuming not i).
ii) concerns me; it would render the GSC help docs useless for a start.
iii) would be ok if we come up with a way of splitting the testdata_help between the repos. But it's not obvious to me how to do that neatly.

@butlerpd butlerpd merged commit f7ece5f into release_0.9.0 Feb 27, 2024
18 checks passed
@butlerpd butlerpd deleted the update_example_data branch February 27, 2024 14:24
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