https://github.com/SPAAM-community/AncientMetagenomeDir/pull/877 his is the problematic PR
*Thread Reply:* Hmm okay, but I could reproduce the error with the main.py script. I fixed it, but now other things turn up.
*Thread Reply:* It's alright for e.g. the ancientmetagenome-hostassociated:
*Thread Reply:* But for the errorneous ancientsinglegenome-hostassociated we get this. Maybe @Maxime Borry you can check what's happening with the logging output?
*Thread Reply:* With "fixing" I mean turning all <a href="http://console.logging.info">console.logging.info</a>() into console.log() in the main.py. Not worth a PR bc it's still not doing what it should I guess π¬
*Thread Reply:* Oh man OK, so my various attempts of installing were always working then π
*Thread Reply:* Yes, I was so confused what exactly was your problem, but yeah. You only ran AMDirT test-dataset and no AMDirT filter just to verify whether it was a faulty installation or faulty code?
*Thread Reply:* Just test-dataset as tat's what I was tryng to run
*Thread Reply:* Oh, the rich API chanded a bitβ¦
@James Fellows Yates You are using the AMDirT dev version, as you posted here? So then you will be affected to the changes since the last master branch merge.
}
Hey @channel , If I get a review of https://github.com/SPAAM-community/AMDirT/pull/69 that would be great. So we can start the hackaton on a more solid code base π
*Thread Reply:* I'll have more time on Friday. Will try the tool and give feedback on Friday afternoon!
*Thread Reply:* @James Fellows Yates Can you post it in the PR, with the associated schema and dataset files ?
*Thread Reply:* Though I suspect the solution to this is just AMDirT validate -v --multi_values archive_data_accession ancientmetagenome-hostassociated_libraries.tsv ancientmetagenome-hostassociated_libraries_schema.json
*Thread Reply:* Iβll remove the default value here https://github.com/maxibor/AMDirT/blob/c0d7bb028048f4c6fd1f4e9e9306b9b041924f01/AMDirT/cli.py#L55 and here https://github.com/maxibor/AMDirT/blob/c0d7bb028048f4c6fd1f4e9e9306b9b041924f01/AMDirT/validate/application/__init__.py#L29
Meeting: https://app.gather.town/app/PlXjb0deog0B4JCq/spaam-community
AMDirT github repo: https://github.com/SPAAM-community/AMDirT/
AncientMetagenomeDir Github repo: https://github.com/SPAAM-community/AncientMetagenomeDir/
*Thread Reply:* could use a quick chat to see if the fix i have for column dtypes (mandating dtypes when parsing into pandas df) has behavior that is useful for the tool or not!
*Thread Reply:* https://github.com/SPAAM-community/AMDirT/issues/57
*Thread Reply:* @Maxime Borry βοΈ
*Thread Reply:* @Ian Light @Maxime Borry I just want to jump in by saying NA comes from my R background (I think)
*Thread Reply:* I dont mind changing the 'no-value avaiable' type if it would help wiht python
*Thread Reply:* anyone have any experience getting json comparison to view python <class NoneType> as instance of null?
*Thread Reply:* Hmm, when a pandas export is being exported to json, NaN should be exported as null https://pandas.pydata.org/docs/reference/api/pandas.DataFrame.to_json.html
*Thread Reply:* it is rather an instance of <class 'pandas._libs.missing.NAType'>
*Thread Reply:* due to the <pd.Int64> class (which can deal with missing values)
*Thread Reply:* maybe worth trying to mess around with default_handler in the pd.DataFrame.to_json
*Thread Reply:* hmmm OK so it does convert it correct to null
*Thread Reply:* But then json.load converts it to None again
*Thread Reply:* and the comparison fails duirng the jsonschema check due to the following conversion: https://docs.python.org/3/library/json.html#json-to-py-table (eg null --> None encoding in python, but jsonscheam validator is looking for 'null'
*Thread Reply:* will look more into this tomorrow... the end is in sight!
*Thread Reply:* The end of the tunnel is near @Ian Light!
Anyone having errors of connection when downloading the whole lot of samples?
*Thread Reply:* Eek, have you been repeatedly downloading stuff repeatedly? You can get blocked if you make too many connection requests
*Thread Reply:* hmmmm like I tried with 3 or 4 samples twice before
*Thread Reply:* Not the same error, but I got this one when trying to generate the download script for all host associated metagenomes
*Thread Reply:* but now that I wanted to try downloading the whole host-associated download script I got the message
*Thread Reply:* OK, please make issues describing the steps you took, and then the error, for @Maxime Borry to look into π
*Thread Reply:* Hey Maria,
This issue is now fixed.
If you run git pull it should now be gone
*Thread Reply:* and now same as Yaxal
*Thread Reply:* Yaxal is doing the issue
*Thread Reply:* Works fine here. Could be an issue with the python version you are using?
*Thread Reply:* (i'm using 3.10 and you 3.9 according to the error message)
*Thread Reply:* hmmm⦠no @Maria Lopopolo also have 3.10
*Thread Reply:* Yes, Maria is also using the same version. I am thinking it can be a limit of how many elements can be processed.
*Thread Reply:* in which table you have the issue ?
*Thread Reply:* We got the error with the metagenome host associated table, but when I tried to replicate with metagenome environmental, which is smaller, it took some time, but I got the expected files
*Thread Reply:* Now I am trying with single genome - host associated
*Thread Reply:* Oh sorry, I was referring to your original screenshot when saying it was fixed
*Thread Reply:* But still, it should now be fixed (there are no more xml accession requests)
*Thread Reply:* It is working smoothly and faster now
I donβt quite understand what the filtering option for range year means when it gives you the AND/OR below.
*Thread Reply:* π sorry to hear that!
*Thread Reply:* Get better soon ! And don't worry, there's plenty of coding left to do π
reposting: Meeting: https://app.gather.town/app/PlXjb0deog0B4JCq/spaam-community
*Thread Reply:* Until 3pm should work also!
*Thread Reply:* I will test it using tox and update the tutorial or leave some common on it, once I finish in next week.
*Thread Reply:* note that I believe tox doesn't test the AMDirT filter functioanlity I believe (right @Maxime Borry?)
*Thread Reply:* I actually wonder if filter is a good verb now I think abou tit...
*Thread Reply:* AMDirT viewer or something?
*Thread Reply:* or Download...
*Thread Reply:* I would vote for AMDir filter or viewer π
*Thread Reply:* I would go for viewer.
*Thread Reply:* It would be hard to automate the tests for an interactive app ;)
*Thread Reply:* I also agree, viewer would be a better fit
*Thread Reply:* Iβm swamped trying to prepare for a deadline on Monday so havenβt gotten to do any useful testing yet! Apologies
*Thread Reply:* I will need to find a a spare 10m of time to test this #98, #94 might take a bit longer
*Thread Reply:* Hi, sorry. I'm overwhelmed by duties now... I can try to do it later, but maybe someone else could prepare it?
*Thread Reply:* no worries, it's up to you. Just as a reminder that people shown as a code contributor on the repo (AMDirt or the Dir) will be included in the paper we are writing in January/Feburary, so if you are interested in that that would be the deadline
And finally @Nikolay Oskolkov for the convert (https://github.com/SPAAM-community/AMDirT/issues/39)
*Thread Reply:* Hi @James Fellows Yates and @Maxime Borry, I am working on the issue 39 now. In order to modify the "convert" function I wanted to refresh how AMDirT works and by running "AMDirT filter" and then selecting "ancientmetagenome-hostassociated" I suddenly got this
*Thread Reply:* Btw I think I correctly installed AMDirT (inlcuding dev-branch), I see 1.4 when I type "AMDirT --version"
*Thread Reply:* Is you dev branch up to date ?
*Thread Reply:* Hmm, thanks @Maxime Borry, it turned out that it was not. Becuse the numerous merge conflicts I believe the easiest would be if I reinstall AMDirT and work on the latest version π
*Thread Reply:* @Maxime Borry and @James Fellows Yates I am working on the "AMDirT convert". As far as I understood the idea was to add an option for the users to choose between converting data tables to "eager" or "fetchngs" formats. Is the conversion (i.e. execution of "AMDirT convert") done in the interactive tool when the users click "download eager format" button? Should I add two separate buttons for "eager" and "fethngs" instead? Or just add another flag to "AMDirT convert" command line tool?
*Thread Reply:* what's missing in the CLI convert tool is a way to get both at once
*Thread Reply:* hence the comment on click multiple options
*Thread Reply:* Context: What I would like is that a user can skip the whole GUI.
So rather than opening the GUI, filtering the table in their webbrowser, and then press 'Download eager',
I would like a user to be able to filter a table themselves (e.g. in R or pandas), and have a CLI tool to do the coverstion.
So basically expose whatever functions that @Maxime Borry already has to a CLI subcommand (convert) rather than just in the GUI
*Thread Reply:* So, the comment that you mentioned is specifically for columns where all valid values are given (in an additional schema), for instance in Longitude for sample data, since the values will only be accepted if the are in range [-180,180], this overrides the acceptance of any null value.
$id": "#/items/properties/longitude", "type": ["number", "null"], "minimum": -180, "maximum": 180, "title": "Longitude", "description": "WGS 84 Longitude Ranges between -180 and 180 (e.g. as used in Google Maps)", "examples": [8.84] }
*Thread Reply:* All other columns which don't have these required value sets or ranges can keep null (and the PR shouldn't mess up with any existing data entries.
*Thread Reply:* the parsing of the pandas df already parses any instances of NA in the .tsv correctly as np.NaN.
*Thread Reply:* but it also accepts other NA like entries as np.NaN: From pandas documentation By default the following values are interpreted as NaN: '', '#N/A', '#N/A N/A', '#NA', '-1.#IND', '-1.#QNAN', '-NaN', '-nan', '1.#IND', '1.#QNAN', '<NA>', 'N/A', 'NA', 'NULL', 'NaN', 'n/a', 'nan', 'null'.
*Thread Reply:* Ok, this is clearer to me now - thanks! So what would I need to do/change then (if you can give me a single check list, my brain capacity is rather low atm, so I assume I'm dumb)
*Thread Reply:* so I think keeping NA for legibility of the tsv is what we should do
*Thread Reply:* Across all columns?
*Thread Reply:* Or just numeric?
*Thread Reply:* as long as the option for a string-column which can accept NA has 'null' as accepted value then it is OK
*Thread Reply:* ToDo (outside of the scope of the current PR): @Maxime Borry let me know if you think I am missing something
- For schemas: if list of accepted values known or range of accepted values known remove accepted value type 'null' for clarity
- For tsv updating instruction (adding new entries), indicate that all missing values should be set as NA.
*Thread Reply:* just to clarify, in the case of the latitute, if the schema checking encounters a NaN, (read 'null' instance), since it is not in range [-180,180] it throws an error.
*Thread Reply:* 1. Ok, so remove null from everywhere in the schemas
- Change all numeric AND character 'missing' value to
NA
*Thread Reply:* (to simplify even further for myself)
*Thread Reply:* 1. not everywhere, just in places where there is a known set of values (eg PAIRED or SINGLE for library layout) or range of accepted values (eg [-180,180] in case of lat/long)
*Thread Reply:* anywhere where $ref occurs in the schema should not accept 'Null'
*Thread Reply:* 1. Remove null from any schema entry that has a enum fixed-list or numeric range
- Change docs and TSV so that 'missing' value is
NAfor ALL columns
*Thread Reply:* Will try to do this before 11 β€οΈ
*Thread Reply:* I'll ping you for review though
*Thread Reply:* (as I don't trust myself)
*Thread Reply:* @Ian Light can I test the schema off your PR (the version in the PR is working, I mean?)
*Thread Reply:* i'm wondering if we should just remove null everywhere, if we are replacing with NA that's a string anyway, no?
*Thread Reply:* Ah but then for the integers with no range that won't work...
*Thread Reply:* We will see, trying to install your branch now π
*Thread Reply:* Is a single-bounded range included ?
*Thread Reply:* As in there is a minimum value but no maximum?
*Thread Reply:* Ok I am confused then
*Thread Reply:* So for example, I've removed null as an option for latitude and longitude
*Thread Reply:* DerSarkissian2016 2016 10.1111/1755-0998.12679 Unknown NA NA Unknown faunal,microbial Unknown NA quahog2_Quahog 100 10.1111/1755-0998.12679 shell shell NA ENA PRJEB20113 ERS1624459
This line reports an error
*Thread Reply:* Or is the assumption is that that that e.g. those two columns MUST have that information?
*Thread Reply:* So maybe to rephrase the question:
> Even if 'null' is accepted value, the required fields will cause error to be thrown for this entry. I recommend removing all instances of accepted type 'null' where full list of accepted values is known. What happens if I still want to retain 'missingness' for a integer/number column?
*Thread Reply:* in a meeting, will reply in 30 min
*Thread Reply:* for retaining missingness for columns with known set of values, we would need to remove the forcing the known set with a schema (at least that was the behavior I observed in my tests, when the additonal schema for accepted values was given, null/NA entries would throw errors)
*Thread Reply:* I'll need to think about it hten
*Thread Reply:* (thanks for your patience)
*Thread Reply:* Iβve had another look at the PR, and made some changes. AFAIK, the only thing that will need to be changed in AncientMetagenomeDir is the multi-types column in the schema. The rest works as expected. @James Fellows Yates
*Thread Reply:* Ok, I'll try and look next week again then
*Thread Reply:* Do the changes now allow a form of 'missing data' values for integer columns, for example?
*Thread Reply:* (like lat:Lon)
*Thread Reply:* of course. Nothing is changed on this side
*Thread Reply:* OK! I have a branch with the updated shcema/sheets (only problematic row was download_sizes it seems)
*Thread Reply:* So once we ready to release AMDirT can merge in that change too
*Thread Reply:* otherwise look good π
*Thread Reply:* Thanks @Ian Light @Maxime Borry!
*Thread Reply:* Will do it first thing Monday
*Thread Reply:* Error π left on the PR as a comment
*Thread Reply:* If you replace mid-december with mid-january, then you have my :largegreencircle:
*Thread Reply:* Do you have an issue with the current AncientMetagenomedir datasets to block release of the metadata?
*Thread Reply:* Oups, I misread that, sorry. Nothing against a release of AncientMetagenomeDir (I thought AMDirT)
*Thread Reply:* Ok cool!
Yes, I would say: theDir release in end of December, AMDirT release end of January, paper writing Feburary, Submission in March
*Thread Reply:* No worries! Hope for good reasons!
*Thread Reply:* Awesome π
*Thread Reply:* Do you have an approx ETA based on your capacities? Just wondering if we should wait for this or not before first release (cc @Maxime Borry)
*Thread Reply:* The next two weeks are going to be very busy for me. I could try to finish everything on the 10th
*Thread Reply:* screen cap for the whole amdirt filter command/GUI is done now, along with the individual snippets.
*Thread Reply:* not sure if you want me to find a place for them already in the github or just deposit them somewhere
*Thread Reply:* how big are the files?
*Thread Reply:* i could also compress them/output lower quality .mp4 files
*Thread Reply:* Yeah they are sort of a bit too chunky for github (personally), what about putting them on the SPAAM youtube channel and embeddeding?
*Thread Reply:* Do you think that would work?
*Thread Reply:* seems reasonable, can you send creds for uploading?
*Thread Reply:* can you send me verification code?
*Thread Reply:* are you in?
*Thread Reply:* i will upload all the relevant videos and set them to unlisted
*Thread Reply:* (so they are sharable/viewable only with a link)
*Thread Reply:* Nah it's fine I think
*Thread Reply:* Well actually
*Thread Reply:* we can list the mlater
*Thread Reply:* sure -- i can also list the full video, and not the short snippets
*Thread Reply:* I'm not fussy
*Thread Reply:* OK they are all up on the youtube
*Thread Reply:* i can maybe spend a bit of time on monday putting the embedding into the walkthrough on github, but today i am done π
*Thread Reply:* That would be awesome!
*Thread Reply:* hmm so it seems that with youtube videos, the best we can do is embed an image of the youtube video preview, which then acts as a hyperlink to the actual video..
we can't have an embedded youtube video that actually plays within the github markdwon
*Thread Reply:* Not even with HTML?
*Thread Reply:* But I guess it would be fine anyway
*Thread Reply:* https://stackoverflow.com/a/69423002/11502856
*Thread Reply:* oh yes i see that html can do it
*Thread Reply:* hmm unless i am misunderstanding how to put html code into markdowns it doesnt seem to work (for reference I just copy and pasted the embedding from youtube into the markdown
*Thread Reply:* You will need to render the actual website, not just the prview
*Thread Reply:* So I think make html and then load the HTML (might require extra sphinx/readtheocs packages)
*Thread Reply:* Really sorry it's taken me so long to get to check your PR @Ian Light - apparntly I didn't get a notification on my fork ,and I was expecting it o nthe main fork π
*Thread Reply:* will check now
*Thread Reply:* no worries -- i just put it up this week π
By the way, have you seen https://scribehow.com/ for making a GUI based documentation ? (Itβs free for web apps)
@Maxime Borry I just saw streamlit supports multiple pages: https://docs.streamlit.io/library/get-started/multipage-apps/create-a-multipage-app
Do you know if the data can be linked between each other?
@channel here is a 80% completed first drraft: https://sharelatex.gwdg.de/read/rtccfgfqpjdh
I am missing figure/table ideas, any suggestions?
Current ideas:
β’ Stats of update of number of samples etc. since the original AncientMetagenomeDir paper β’ A Workflow diagram and/or screenshot? of how to use AMDirT
*Thread Reply:* Hmmm that's true, but I worry that that might go stale quite quickly, community involvement fluctuates a lot and coudl die out (I have to do a lot of poking/pushing atm)
*Thread Reply:* also what i was concerned about π
*Thread Reply:* But might be nice to have as a little badge or something π¬
*Thread Reply:* In the editor on the left? Weird, I still cannot edit (using Firefox, refreshed page, closed/re-openend and re-compiled)
*Thread Reply:* Can you leave comments?
*Thread Reply:* No. Maybe one needs to be logged in to Overleaf? I tried to with SSO but in the end did not get access (uni Jena has no permission, HKI is missing certain login information) π¬
*Thread Reply:* Trying with HKI account
*Thread Reply:* Ahhh nevermind, I saw your new link which works!!
Track changes is ON if you want to read leave comments/suggestions: https://sharelatex.gwdg.de/9559758929ygqwcysfccxz
*Thread Reply:* Me neither, cannot add comments, the "Add comment" button doesn't pop up when selecting text. In the current line 267 there is an incomplete subsentence: > ..., and whether aDNA degradation. You might want to finish that sentence βοΈ
*Thread Reply:* i added the end to that sentence π
*Thread Reply:* Thank you both!
yeah I'm not sure about the future directions section. Is it even worth it at all?
*Thread Reply:* Hi, I follow up here. I think I was happy to read that we think to integrate exact radiocarbon dates where possible π (maybe even with two formats BP/CE to choose from).
*Thread Reply:* Would you like to help?
*Thread Reply:* @Diana has already started gathering it but soon will need help with reviews
*Thread Reply:* Will let you know when it's ready
*Thread Reply:* Lol fell in your trap again β οΈ
*Thread Reply:* It will need a team effort again, but for this I want to pull in just patho peeps
*Thread Reply:* You need an personal email invite with an account to be able to leave comments π«€
*Thread Reply:* No there is a public link as well, I just sent teh read only link the first time around
*Thread Reply:* I didn't check in detail, but some results fields are renamed/removed
*Thread Reply:* @Maxime Borry do you also want to announce on #general?
*Thread Reply:* Let's wait until next week π
*Thread Reply:* break stuff
@Maxime Borry I've been working with @Diana on another the Dir extension (including radiocarbon dates)
I've built a schema and tried testing it against a dummy-ish dataset here:
https://github.com/jfy133/AncientMetagenomeDir/pull/2
When I run it locally and on the GHA it fails with some URL resolver error (I think, but could be wrong, long traceback)
File "/opt/hostedtoolcache/Python/3.9.17/x64/lib/python3.9/site-packages/jsonschema/validators.py", line 914, in resolve_from_url
raise exceptions.RefResolutionError(exc)
jsonschema.exceptions.RefResolutionError: Expecting value: line 1 column 1 (char 0)
any idea what could be causing that?
<a href="https://github.com/jfy133/AncientMetagenomeDir">jfy133/AncientMetagenomeDir</a>
*Thread Reply:* Should also repost in #ancientmetagenomedir!
Thoughts on updated diagram?
*Thread Reply:* Cool! Just need to put white background layer behind this. Typo in top right: "Revelance" -> "Relevance"
Not anywhere so far, this is due to this issue with how Firefox security settings handles the loading of remote resources https://github.com/whitphx/stlite/issues/556
<a href="https://github.com/whitphx/stlite">whitphx/stlite</a>
